Closed Bug 330890 Opened 15 years ago Closed 15 years ago
unify does not preserve the execute bit
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 run-mozilla.sh, redo-prebinding.sh, etc.
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.
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 is wrong: + # Wraps $FileAttrCache->lstat(), returning the type bits from the st_mode should say "mode bits"
Tested successfully on maya-1.8.0-Uni.
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?
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 on attachment 215470 [details] [diff] [review] v2 I'm sold. Ship it.
Attachment #215470 - Flags: review?(preed) → review+
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 approval18.104.22.168 for the 1.8.0 branch.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 215470 [details] [diff] [review] v2 approved for 22.214.171.124, a=dveditz for drivers
Attachment #215470 - Flags: approval126.96.36.199? → approval188.8.131.52+
Fixed on 1_8_0. preed, it's now in your hands to move the tag on unify or whatever.
*** Bug 331280 has been marked as a duplicate of this bug. ***
331280 is not a dup of this bug.
I landed this on 1_8 earlier today after talking with preed, in case y'all decide to push a universal 2a1.
You need to log in before you can comment on or make changes to this bug.