If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

partial update mar creation doesn't preserve +x bit on new binaries (auto-update from 2009-12-14 to 2009-12-15 m-c nightly creates new file "mozilla-runtime" as non-executable)

RESOLVED FIXED

Status

Release Engineering
General
RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: dholbert, Assigned: nthomas)

Tracking

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
STEPS TO REPRODUCE:
 1. Download & extract the mozilla-central nightly build from 2009-12-14 [1]
 2. Run the build in a fresh profile, and do Help | Check for Updates
 3. Download the update when prompted, and restart Firefox to allow it to install
 4. Examine the permissions on the new file "mozilla-runtime"

ACTUAL RESULTS:
 mozilla-runtime is *not* executable

EXPECTED RESULTS:
 mozilla-runtime should be executable, as it is in a 2009-12-15 build.[2]


[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009/12/2009-12-14-03-mozilla-central/firefox-3.7a1pre.en-US.linux-i686.tar.bz2

[2] http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009/12/2009-12-15-03-mozilla-central/firefox-3.7a1pre.en-US.linux-i686.tar.bz2

This issue is one cause of the hang in bug 535077, since exec() fails for non-executable files.
(Reporter)

Comment 1

8 years ago
Note: I'm assuming this is a bug in our update code path (either on the server side, generating the updates, or on the client side, receiving and applying the updates).

Not sure what the correct component is for it.  CCing ted and rob_strong, in the hopes that one of them might know.
See Also: → bug 535077
Is it non-executable in the tar.bz2 as well?
(Reporter)

Comment 3

8 years ago
In link [2] from comment 0, it's executable.

In other words, a freshly-obtained mozilla-central nightly has it as executable.  But an old nightly, updated via "Help | Check for Updates", gets a non-executable copy.  (They're identical in every other way, though -- "diff" says they're the same)
(Reporter)

Updated

8 years ago
Summary: Auto-update from 2009-12-14 to 2009-12-15 m-c nightly creates new file "mozilla-runtime" as non-executable → Auto-update from earlier than 2009-12-15 m-c nightly creates new file "mozilla-runtime" as non-executable
So, link [1] does not have the executable bit set?

Complete updates and new files should retain the permissions set when packaged whereas partials retain the existing permission. I believe I have this covered for both Mac and Linux using xpcshell tests and the updater binary. So, if the first update containing that file didn't have the executable bit set the updater wouldn't set it. Not sure if this is in fact what happened but if it did one way to fix this is to have releng force a complete copy of the binary in the next update or only offer a complete update.
(In reply to comment #4)
> So, link [1] does not have the executable bit set?
Sorry, I thought that build had the mozilla-runtime as well.
(Reporter)

Comment 6

8 years ago
Sorry, I should have been clearer in original description. "mozilla-runtime" is a new file added for out-of-process plugins.  It first appears in the 2009-12-15 nightly.
nah, I knew that but didn't notice the date on link 1.

cc'ing nthomas and bsmedberg if for no other reason than to give them a heads up.

btw: update xpcshell test_0110_general.js tests permissions on file replacement (new files with 0755 and 0644 that replace existing files with different perms) which I believe is the same for new files and update xpcshell test_0110_general.js tests adding a new file with 0644.

I would also expect to see this happening on Mac OS X if it were update code.
(Assignee)

Comment 8

8 years ago
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009/12/2009-12-15-03-mozilla-central/firefox-3.7a1pre.en-US.linux-i686.complete.mar gives mode 755 for mozilla-runtime

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009/12/2009-12-14-03-mozilla-central/firefox-3.7a1pre.en-US.linux-i686.partial.20091214030830-20091215030948.mar gives mode 644.

So it's a bug in the partial generation. In fact, I'll assert the problem is that bzip2 doesn't preserve the permissions at 
 http://hg.mozilla.org/mozilla-central/file/tip/tools/update-packaging/make_incremental_update.sh#l168
In place compression would preserve, redirecting like that doesn't. So all new files will get the 644 mode, and in the same partial libmozsqlite3.so is also affected.
Component: General → Release Engineering
Product: Core → mozilla.org
QA Contact: general → release
Version: Trunk → other
(Assignee)

Comment 9

8 years ago
This is probably a suitable hack to fit into the style of this file
  chmod `stat -c %a "$newdir/$f"` "$workdir/$f"
The stat arguments may not be very portable though.

Updated

8 years ago
Blocks: 531142
(Reporter)

Updated

8 years ago
blocking2.0: --- → ?
(Reporter)

Comment 10

8 years ago
IMHO this bug needs to block 1.9.3 (or whatever next release includes OOP plugins).  But there aren't any flags that I can use to request that right now...

Comment 11

8 years ago
It blocks bug 531142, so you're covered.
Assignee: nobody → dolske
I'll see if the fix suggested in comment 8 is all that's needed here.
test -x "$newdir/$f" && chmod +x "$workdir/$f"

might work if all we care about is the executable bit

chmod --reference="$newdir/$f" "$workdir/$f"

might also work
(In reply to comment #8)
> So it's a bug in the partial generation. In fact, I'll assert the problem is
> that bzip2 doesn't preserve the permissions at 
> http://hg.mozilla.org/mozilla-central/file/tip/tools/update-packaging/make_incremental_update.sh#l168
bsmedberg reports that this is blocking OOP work in bug#531142. Urgh - there
are only 3 changes to that file during 2009 - how long ago exactly did that
break, or has this always been a problem? 


> In place compression would preserve, redirecting like that doesn't. So all new
> files will get the 644 mode, and in the same partial libmozsqlite3.so is also
> affected.

bsmedberg/dholbert: if you do a new install m-c nightly from *after*
2009-12-15, is this still a recurring problem, or is there something specific
to the 2009-12-15 date?
(Reporter)

Comment 15

8 years ago
(In reply to comment #14)
> bsmedberg/dholbert: if you do a new install m-c nightly from *after*
> 2009-12-15, is this still a recurring problem, or is there something specific
> to the 2009-12-15 date?

joduinn: see comment 3.  The significance of 2009-12-15 is: that's the nightly where this file ("mozilla-runtime") first appears.  Auto-updates from before that to after that will create this file as non-executable, and that's the problem here.

A fresh install after that date will create the file just fine (executable), as noted in comment 3.

Comment 16

8 years ago
It's specific to a partial update, so the nightly from 2009-12-14 to 2009-12-15. But it will reproduce when we do the first beta and the first release and have partials for those.
(Assignee)

Comment 17

8 years ago
We use different code for releases, eg
 http://hg.mozilla.org/releases/mozilla-1.9.2/file/UPDATE_PACKAGING_R10/tools/update-packaging/make_incremental_updates.py
For new files, create_add_patch_for_file() calls to copy_file(), which shutil.copy2 to duplicate the (compressed) file from the complete mar. Since that preserves the metadata I don't think we'll hit this issue for releases.

This bug affects new executables in nightly builds only.
Blocks: 535077
See Also: bug 535077
(Assignee)

Comment 18

8 years ago
This does not block bug 535077.
No longer blocks: 535077
(Reporter)

Comment 19

8 years ago
Ok -- marking that bug as "see also", then, since that bug is one reason this bug matters.
See Also: → bug 535077
(In reply to comment #18 & #19)

I removed it from the "See Also" because this bugzilla installation's bugs don't really belong there per the description of that field. It's meant for other bugzilla installations, this one's belong in the "Depends" or "Blocks" field...

Comment 21

8 years ago
ping? We're coming up on an alpha/beta with possible updates next week, I'd like to get this landed and baked somehow.
(Assignee)

Updated

8 years ago
Summary: Auto-update from earlier than 2009-12-15 m-c nightly creates new file "mozilla-runtime" as non-executable → Nightly partial updater doesn't preserve +x bit on new binaries (auto-update from 2009-12-14 to 2009-12-15 m-c nightly creates new file "mozilla-runtime" as non-executable)
(Assignee)

Comment 22

8 years ago
Created attachment 422933 [details] [diff] [review]
Fix nightly partial generation

Using this patch I can create a partial between 2009-12-14/15 nightlies that preserves the 755 permission on mozilla-runtime and libmozsqlite3.so. There are no differences in mode or file content between the 14th build updated with that partial and the 15th build.

Looks like more work than you'd really need to do, except that stat isn't available in MozillaBuild, and linux & mac differ sufficiently to make it a pain to use them; calling out to perl/python defeats the point of doing this in shell originally.
Attachment #422933 - Flags: review?(ccooper)
Bah, I'm too slow. :)

FWIW, bug 529464 will be changing the updater to always treat the .mar files as the canonical source for file permissions. But AIUI the only problem right now is that the mars are getting the wrong permissions for ADD items in the manifest?
Assignee: dolske → nrthomas
(Assignee)

Comment 24

8 years ago
(In reply to comment #17)
> ... I don't think we'll hit this issue for releases.

Confirmed by calling make_incremental_updates.py against the mars for the 14th/15th, applying the mar and comparing files and modes.

(In reply to comment #23)
> FWIW, bug 529464 will be changing the updater to always treat the .mar files as
> the canonical source for file permissions. 

Currently the patch files within the mar are 644, so that might cause some interesting issues.

> But AIUI the only problem right now
> is that the mars are getting the wrong permissions for ADD items in the
> manifest?

As filed it's when binary files are added to the package. Forcing a file is also an ADD in the manifest, or if the patch ends up larger than the file (small files + bz2 header). And sure enough there are other instances of the code pattern I'm changing, will update the patch.
(Assignee)

Comment 25

8 years ago
Created attachment 422945 [details] [diff] [review]
Fix nightly partial generation, v2

Covers the other two cases as well.
Attachment #422933 - Attachment is obsolete: true
Attachment #422945 - Flags: review?(ccooper)
Attachment #422933 - Flags: review?(ccooper)

Updated

8 years ago
Attachment #422945 - Flags: review?(ccooper) → review+
Comment on attachment 422945 [details] [diff] [review]
Fix nightly partial generation, v2

Worthwhile putting this into a function, since it is used in 3 places?

Also, does the python variant of this script have the same issue?
(Assignee)

Comment 27

8 years ago
Created attachment 423287 [details] [diff] [review]
Fix nightly partial generation, v3

Apologies for the churn here but I noticed that when we create a complete mar (for both releases & nightlies) we call make_full_update.sh, which uses copy_perm() in common.sh. That sets mode 755 or 644 in mar depending on the executable bit of the source file. I think we should use the the same mechanism for nightly partial creation too, just to keep the handling the same.

The release case (make_incremental_updates.py) is verified unaffected at comment #24.
Attachment #422945 - Attachment is obsolete: true
Attachment #423287 - Flags: review?(ccooper)

Updated

8 years ago
Attachment #423287 - Flags: review?(ccooper) → review+
(Reporter)

Comment 28

8 years ago
Looks like IPC plugins just switched to default-enabled...
   http://hg.mozilla.org/mozilla-central/rev/f54bb3222492
...which means tonight's nightly update will expose a whole swath of nightly testers to this bug, unless the patch here lands ASAP.

nthomas, can you check this in and make sure it gets used in generating tonight's nightly update?  (if you haven't already)
I suspect that the nightly might also need to do a force add of the new file since we preserve existing permissions for partial updates or only offer complete update to fix this for all nightly users.

Comment 30

8 years ago
Since this was added I hope/expect that most nightly users have taken a day off somewhere, which would give them a full update instead of a partial, and we've done multiple nightlies on a couple days. I really don't think many people are going to see this on the nightly channel now.
I agree... just pointing out the possibility and that a simple fix would be to offer only a complete update for the one nightly.
(Assignee)

Comment 32

8 years ago
I'll get it landed today. 

Only offering a complete isn't straightforward, but we could we just force mozilla-runtime in the partial. Also libmozsqlite3.so although +x doesn't seem to be required, so it would just be for consistency.
(Assignee)

Comment 33

8 years ago
Created attachment 423901 [details] [diff] [review]
As landed in CVS

Checking in make_incremental_update.sh;
/cvsroot/mozilla/tools/update-packaging/make_incremental_update.sh,v  <--  make_incremental_update.sh
new revision: 1.16; previous revision: 1.15
done

Updated prometheus-vm and staging-nightly-updates (including the README at /builds/nightly-partial-generation/mozilla-checkout). I'll back out the change to requested_forced_updates tomorrow.

Landings on the branches to follow.
(Assignee)

Comment 34

8 years ago
Comment on attachment 423901 [details] [diff] [review]
As landed in CVS

>Index: make_incremental_update.sh
>-requested_forced_updates='components/components.list Contents/MacOS/components/components.list'
>+requested_forced_updates='components/components.list Contents/MacOS/components/components.list mozilla-runtime libmozsqlite3.so'

Reverted this change in rev 1.17, and partial generators updated.
(Assignee)

Comment 35

8 years ago
Comment on attachment 423287 [details] [diff] [review]
Fix nightly partial generation, v3

Checked into active branches:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0555a0269dca
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ce136686a20a

m-c is closed for the Alpha1 freeze.
Attachment #423287 - Flags: checked-in+
(Assignee)

Comment 36

8 years ago
Comment on attachment 423287 [details] [diff] [review]
Fix nightly partial generation, v3

http://hg.mozilla.org/mozilla-central/rev/d72639947a60
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Did this get a test added in the partial-update test setup?  I don't see it in the patches, but I don't really know what I'm looking for tbqh.

We're looking at a change to the naming of the executable, and just want to make sure that we don't trip over old bugs again when we do so.
Not sure about the tests for mar creation but there are xpcshell tests that verify that the client side preserves the permissions provided by the mar.
Summary: Nightly partial updater doesn't preserve +x bit on new binaries (auto-update from 2009-12-14 to 2009-12-15 m-c nightly creates new file "mozilla-runtime" as non-executable) → partial update mar creation doesn't preserve +x bit on new binaries (auto-update from 2009-12-14 to 2009-12-15 m-c nightly creates new file "mozilla-runtime" as non-executable)

Updated

7 years ago
blocking2.0: ? → betaN+
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.