As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact
Last Comment Bug 330890 - unify does not preserve the execute bit
: unify does not preserve the execute bit
: fixed1.8.0.2, fixed1.8.1
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: PowerPC Mac OS X
: -- critical (vote)
: ---
Assigned To: Mark Mentovai
: Gregory Szorc [:gps]
Depends on:
  Show dependency treegraph
Reported: 2006-03-17 16:10 PST by Mark Mentovai
Modified: 2006-11-10 12:14 PST (History)
2 users (show)
dveditz: blocking1.8.0.2+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Preserve the x-bit when copying files (8.47 KB, patch)
2006-03-17 17:18 PST, Mark Mentovai
no flags Details | Diff | Splinter Review
v2 (9.87 KB, patch)
2006-03-17 20:01 PST, Mark Mentovai
mozpreed: review+
mark: approval‑branch‑1.8.1+
dveditz: approval1.8.0.2+
Details | Diff | Splinter Review

Description User image Mark Mentovai 2006-03-17 16:10:57 PST
When producing a universal binary, unify does not preserve the execute bit on non-Mach-O files that it creates.  It MUST.

This bug is causing un-executable scripts to be packaged for,, etc.
Comment 1 User image Mark Mentovai 2006-03-17 17:18:21 PST
Created attachment 215459 [details] [diff] [review]
Preserve the x-bit when copying files

File::Copy::copy doesn't preserve the x-bit when it makes a copy to a new file like the "cp" command does.  This change makes unify set all of the x-bits allowed by the umask for a file that's being copied if any x-bit is set on either source file.  The structure of this change also strengthens unify against a possible race condition (which would have never occurred in our use case) and prevents it from deleting files it didn't create.
Comment 2 User image Mark Mentovai 2006-03-17 17:20:20 PST
Don't worry about the permissive modes, it's got the same level of w-bits that we were already getting implicitly.  The tinderboxen run with umask 0, but we don't actually ship anything like that, the dmg packager (pkg-dmg) fixes up permissions in a private copy it makes of the files before putting anything into a dmg.
Comment 3 User image Mark Mentovai 2006-03-17 17:22:51 PST
Comment is wrong:

+  # Wraps $FileAttrCache->lstat(), returning the type bits from the st_mode

should say "mode bits"
Comment 4 User image Mark Mentovai 2006-03-17 17:33:24 PST
Tested successfully on maya-1.8.0-Uni.
Comment 5 User image J. Paul Reed [:preed] 2006-03-17 19:06:18 PST
Comment on attachment 215459 [details] [diff] [review]
Preserve the x-bit when copying files

Is there a reason you create an empty file over just chmod()ing (as appropriate) the file after you've File::copy()ed it?
Comment 6 User image Mark Mentovai 2006-03-17 20:01:31 PST
Created attachment 215470 [details] [diff] [review]

There's a good reason:

Doing the open early with O_CREAT|O_EXCL guarantees that unify won't trample existing files.  This was the intended behavior from the very beginning, but I cheated for regular files and implemented the check as a stat call to see if the target file existed.  This is the race I talked about above and in the code section that I'm removing.

The only functional change in this version of the patch is that I've added the lIsExecutable() check in makeUniversalFile(), because I realized that it's possible to have Mach-O files that properly don't have any x-bits set: object and static archive files.  I also noticed that lipo sometimes ignores the umask, so I've added a late chmod($mode & ~umask()) for universal files too.  Now, entire trees created by unify should have the appropriate bits set and respect the umask.
Comment 7 User image J. Paul Reed [:preed] 2006-03-18 00:02:44 PST
Comment on attachment 215470 [details] [diff] [review]

I'm sold.

Ship it.
Comment 8 User image Mark Mentovai 2006-03-18 06:32:47 PST
Checked in on trunk.

Not checked in on 1.8 yet because a "slushy freeze" is in effect on that branch, and this is not critical there unless 2a1 will ship as a universal binary.

Awaiting approval1.8.0.2 for the 1.8.0 branch.
Comment 9 User image Daniel Veditz [:dveditz] 2006-03-20 11:18:12 PST
Comment on attachment 215470 [details] [diff] [review]

approved for, a=dveditz for drivers
Comment 10 User image Mark Mentovai 2006-03-20 11:38:40 PST
Fixed on 1_8_0.

preed, it's now in your hands to move the tag on unify or whatever.
Comment 11 User image Dave Liebreich [:davel] 2006-03-21 19:20:21 PST
*** Bug 331280 has been marked as a duplicate of this bug. ***
Comment 12 User image Dave Liebreich [:davel] 2006-03-21 19:25:42 PST
331280 is not a dup of this bug.
Comment 13 User image Mark Mentovai 2006-03-21 19:47:32 PST
I landed this on 1_8 earlier today after talking with preed, in case y'all decide to push a universal 2a1.

Note You need to log in before you can comment on or make changes to this bug.