Closed Bug 473236 Opened 11 years ago Closed 11 years ago

Remove executable bit from files that don't need it

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(8 files, 5 obsolete files)

199.05 KB, text/plain
Details
22 bytes, text/plain
Details
393 bytes, text/plain
Details
502.30 KB, patch
dholbert
: review+
Details | Diff | Splinter Review
882 bytes, text/plain
Details
1.79 KB, text/plain
Details
1.08 KB, patch
Details | Diff | Splinter Review
4.38 KB, patch
Details | Diff | Splinter Review
Currently, there are 3881 files in mozilla-central that have their executable bit set.  The vast majority of those files are clearly not intended to be executed.

I'm filing this bug on un-setting the executable bit for the files that clearly don't need it.
Note:  I tried to fix this for a bunch of files last July, as
   http://hg.mozilla.org/mozilla-central/rev/8a99db3f8680
but the change was accidentally reverted in someone's merge a day or so later.
Here's an alphabetical list of file extensions whose files I'm pretty sure shouldn't be executable.  Next to each extension, I've included the count of offending executable files in mozilla-central.

You can use this list to clear the executable bits by running the following command (on Ubuntu Linux, and probably on Mac as well):
   for x in `cat ~/Desktop/extension_counts.txt | cut -f1 -d' '`; do  \
     find . -type f -name "*\.$x" -exec chmod -x {} \; ; done

where "~/Desktop/extension_counts.txt" is the name of this saved attachment.

That command removes the executable bits from 3344 files.
And here's a list of two no-extension filenames which shouldn't be executable -- "README" and "Makefile" -- and the corresponding counts of offending files.

This command will remove the executable bits from all these files:
   for x in `cat ~/Desktop/filename_counts.txt | cut -f1 -d' '`; do  \
     find . -type f -name "$x" -exec chmod -x {} \; ; done

That gets rid of 77 executable files, leaving a total of 460.
Attachment #356591 - Attachment description: list of executable files in mozilla-central → list of all executable files in mozilla-central
Here's a patch resulting from running the commands in my previous two comments.

Since this fix changes file modes, the "patch" command doesn't understand it -- you need to apply it with "hg import", like this:
   hg import --no-commit ~/Desktop/executable_fix.patch
Assignee: nobody → dholbert
I noticed that the ".ex" extension actually was for files that expect to be executed ("hdiutil-expect.ex" and "installdmg-expect.ex").

I'm removing that one from the list of file extensions, and reposting the list & patch.
Attachment #356601 - Attachment is obsolete: true
Here's an updated patch that doesn't strip executable-status from ".ex" files, per my previous comment.
Attachment #356612 - Attachment is obsolete: true
d'oh, I somehow didn't actually remove the "ex" extension from my previous 2 attachments... here's the actual updated list of extensions.
Attachment #356613 - Attachment is obsolete: true
Attachment #356614 - Attachment is obsolete: true
Attachment #356614 - Attachment description: patch v2: allow *.ex to be executable → [ignore -- same as previous patch]
Attachment #356613 - Attachment description: list of file extensions which shouldn't be executable v2 → [ignore -- same as previous list of file extensions]
Attachment #356613 - Attachment filename: extension_counts_v2.txt → extension_counts.txt
Comment on attachment 356619 [details] [diff] [review]
patch v2a: allow *.ex to be executable

bsmedberg, would you be an appropriate reviewer for this?

(I'd recommend mostly looking at attachment 356618 [details] and attachment 356605 [details] -- the patch is long, but it's just generated from those shorter attachments via the "for x in" commands listed above)
Attachment #356619 - Flags: review?(benjamin)
Attachment #356619 - Flags: review?(benjamin) → review+
Here's the patch, regenerated using up-do-date mozilla-central.  Carrying forward bsmedberg's r+.
Attachment #356619 - Attachment is obsolete: true
Attachment #358005 - Flags: review+
Attachment #358005 - Attachment description: patch v2b:updated to current mozilla-central → patch v2b: updated to current mozilla-central
Patch v2b pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/0d6969b2a955
Status: NEW → RESOLVED
Closed: 11 years ago
OS: Linux → All
Hardware: x86 → All
Resolution: --- → FIXED
At ted's suggestion, I ran through the files tweaked by patch v2b, to see if any of them have "#!" in the first line (and hence maybe expect to be executable).

Here's the list, for non-NSS.  (I'm separating NSS out because (a) that just gets periodically dumped from CVS and (b) the potential false-positives there are all pretty uniform -- they're all Makefiles.)
here's the list for NSS.

The command I used to generate these is (newlines added for clarity):
for x in `cat /home/dholbert/Desktop/executable_fix_new.patch
    | grep "diff \-\-git"
    | cut -f3 -d' '`; do
  head -n1 $x | grep "\#\!";
  if [[ $? = 0 ]]; then 
    echo $x; 
  fi;
done
(In reply to comment #13)
> The command I used to generate these is (newlines added for clarity):
(Also -- before running that, I created a symlink called "a" created that points to my mozilla-central source, and ran the command in a's parent directory.  That just meant I didn't have to strip the initial "a" off of the filenames taken from the patch. :) )
Comment on attachment 358447 [details]
potential false-positives, non-NSS

Here are the potential false-positives, arranged in sub-groups:

>#! /bin/sh
>a/build/autoconf/config.guess
>#! /bin/sh
>a/build/autoconf/config.sub

Ted already fixed these in bug 474973.

>#!/bin/sh
>a/build/unix/mozilla-config.in
>#! /bin/sh
>a/nsprpub/build/autoconf/config.guess
>#! /bin/sh
>a/nsprpub/build/autoconf/config.sub
>#!/bin/sh
>a/nsprpub/config/nspr-config.in
>#! /bin/sh
>a/toolkit/crashreporter/google-breakpad/autotools/config.guess
>#! /bin/sh
>a/toolkit/crashreporter/google-breakpad/autotools/config.sub

Looks like the above files expect to have executable status.

>#!/usr/bin/js
>a/testing/tools/pageloader/test/chrome/content/pageloader.js
>#!/usr/bin/js
>a/testing/tools/pageloader/test/chrome/content/report.js

Apparently we run some tests-processing scripts with a command called "/usr/bin/js"? Interesting.  I should probably re-enable those, just to be safe.

>#!/usr/bin/perl
>a/toolkit/mozapps/installer/CFGParser.pm
>#!/usr/bin/perl -w
>a/config/Moz/Milestone.pm

pm = perl library = shouldn't be run directly, right? These don't need to be executable, I'd think.

>#!nmake
>a/embedding/qa/mozembed/makefile.win
>#!nmake
>a/embedding/qa/mozembed/public/makefile.win
>#!nmake
>a/tools/trace-malloc/makefile.win

nmake --> these are for Visual Studio, which is on Windows & doesn't care about exec bits.

>Binary file (standard input) matches
>a/toolkit/themes/winstripe/global/toolbar/spring.gif

And, images don't need executable status, even if they have #! in their first line. :)
This patch re-enables executable status for the files in the top half of comment 15.
And here's a patch re-adding executable status to Makefiles with "#! gmake" in the "nss" folder, because
 - it's conceivable that someone might depend on running them directly
 - their current executable status in mozilla-central will be blown away next time we get a snapshot of NSS anyway.
diff --git a/nsprpub/config/nspr-config.in b/nsprpub/config/nspr-config.in

Note that nsprpub/ is also imported from NSPR's CVS repo.
(In reply to comment #18)
> Note that nsprpub/ is also imported from NSPR's CVS repo.
Ah, good to know -- so any other changes I made there there will get overwritten at next import.  Oh well -- after followup v3a, nspr only had 6 files that my patch tweaked. (2 .c files, 1 each .cpp/h/.tmpl files, and a Makefile.in)  6 more unnecessarily-executable files won't be a huge deal. :)
Landed followup patches with approval from Sheriff Ted.
 Followup Patch v3a: http://hg.mozilla.org/mozilla-central/rev/bdf096d22018
 Followup Patch v3b: http://hg.mozilla.org/mozilla-central/rev/a5ceb7405ca2
Landed another batch of these for test files in /layout:
  http://hg.mozilla.org/integration/mozilla-inbound/rev/26114d0e9609
Removed executable bit from 56 png files that were formerly marked as executable:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/23c638395987
You need to log in before you can comment on or make changes to this bug.