523420 - post_upload.py fails when contrib dirs have contents owned by others

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

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

Attachment: Ignore contrib dirs in post_upload.py

I ran into this while doing the rebuilds for tr for 3.5.4 build3.

Running post-upload command: post_upload.py -p firefox -v 3.5.4 -n 3 --release-to-candidates-dir
Traceback (most recent call last):
  File "/usr/bin/post_upload.py", line 243, in ?
    func(options, upload_dir, files)
  File "/usr/bin/post_upload.py", line 137, in ReleaseToCandidatesDir
    os.chmod(os.path.join(root, d), 0755)
OSError: [Errno 1] Operation not permitted: '/home/ftp/pub/firefox/nightly/3.5.4-candidates/build3/contrib/solaris_pkgadd'
Encountered error while uploading

Essentially post_upload.py tries to chmod the entire candidates dir, but runs into files that it doesn't own under the contribs dirs. This causes the chmod to fail, and prevents the subsequent upload of the MAR file.

I dodged around this during the release by applying the attached patch to the local copy of post_upload.py to avoid touching the contrib dirs. I'm open to other suggestions about how to approach this. It's an edge case to be sure.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Perhaps the download target failed to exclude contrib*/ when sucking builds down ? 

http://hg.mozilla.org/build/tools/file/380d197f23e5/release/signing/download-exclude.list

Comment 2

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

(In reply to comment #1)
> Perhaps the download target failed to exclude contrib*/ when sucking builds
> down ? 

Doesn't look that way. I checked both download-exclude.list files I used when signing (build3 and the tr rebuild) and they both exclude those dirs (and those dirs do not appear on keymaster in the downloaded files).

Comment 3

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

Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.

Comment 4

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

Perhaps just creating the contrib/ dir earlier in the process so we can't get "scooped?"

Comment 6

[[e-mail] (:bhearsum)]

Based on the other work I have planned for the quarter I don't think I'll be getting to this.

Comment 7

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

Ben: is my patch a valid solution here, or do we need something more elaborate?

Comment 8

[[e-mail] (:bhearsum)]

(In reply to comment #6)
> Based on the other work I have planned for the quarter I don't think I'll be
> getting to this.

Please disregard.

(In reply to comment #7)
> Ben: is my patch a valid solution here, or do we need something more elaborate?

I haven't had a chance to really think about this, I'm not sure yet.

Comment 9

[[e-mail] (:bhearsum)]

Coop, your patch seems like it would work and I can't think of any downsides. I'm going to get it landed.

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 407345 [details] [diff] [review]
Ignore contrib dirs in post_upload.py

Catlee points out that this patch has no effect because everything is commented out. I'll write the real one.

Comment 11

[[e-mail] (:bhearsum)]

Attachment: [untested] set correct contrib perms; skip contributed directories

Need to test this in staging still.

Comment 12

[[e-mail] (:bhearsum)]

Attachment: tested, working version

This patch will get us ensuring that contrib dirs have correct permissions and ignore any dirs inside of contrib. tested in staging.

Comment 13

[[e-mail] (:bhearsum)]

Comment on attachment 493430 [details] [diff] [review]
tested, working version

changeset:   1317:317feefa028f

Comment 14

[[e-mail] (:bhearsum)]

I updated the checkouts on stage, staging-stage, and preproduction-stage.