[PATCH] setup: Set mode 644 or 755 on installed files

Kyle Lippincott spectral at google.com
Wed Oct 1 20:16:14 CDT 2014


On Wed, Oct 1, 2014 at 5:40 PM, Mads Kiilerich <mads at kiilerich.com> wrote:

> On 10/01/2014 11:00 PM, Kyle Lippincott wrote:
>
>> # HG changeset patch
>> # User Kyle Lippincott <spectral at google.com>
>> # Date 1412122434 25200
>> #      Tue Sep 30 17:13:54 2014 -0700
>> # Node ID b168f18ef708e5c834eb5ce9f6804c033e769082
>> # Parent  939ce500c92a3dcc0e10815242361ff70a6fcae9
>> setup: Set mode 644 or 755 on installed files
>>
>
> The Scripture says that the subject line should be all lower case.
>

Yeah, mpm mentioned it to me already; he said it could be fixed in flight,
which I assume meant I shouldn't send another patch with just that fixed.


>
> A description that explains a bit about the context and the "why" would be
> nice. The docstring do however seem to explain most of it.
>

This is my first patch, so perhaps a naive process question: should I send
another patch with the capitalization fix and an expanded description, or
is that something that I should expect whomever applies the patch to
correct?


> It is sad that such a minor change requires so much code. But distutils :-(
>

Yeah.  :(


>
> Couldn't we just set umask in this script or in Makefile ... or fail early
> if it is bad?


I think at that point it would be too late, because the files in my repo
will have already had the umask (as of the time of my 'hg clone') applied
to them.  distutils's copy_file defaults to keep_permissions (and
install_lib provides us no way to specify or change this), which means the
umask at the time of running setup.py is, afaict, 100% irrelevant.


>
> /Mads
>
>  diff -r 939ce500c92a -r b168f18ef708 setup.py
>> --- a/setup.py  Mon Sep 29 17:23:38 2014 -0500
>> +++ b/setup.py  Tue Sep 30 17:13:54 2014 -0700
>>
>
>  @@ -375,6 +376,39 @@
>>                                         libraries=[],
>>                                         output_dir=self.build_temp)
>>   +class hginstalllib(install_lib):
>> +    '''
>> +    This is a specialization of install_lib that replaces the copy_file
>> used
>> +    there so that it supports setting the mode of files after copying
>> them,
>> +    instead of just preserving the mode that the files originally had.
>> If your
>> +    system has a umask of something like 027, preserving the permissions
>> when
>> +    copying will lead to a broken install.
>> +
>> +    Note that just passing keep_permissions=False to copy_file would be
>> +    insufficient, as it might still be applying a umask.
>> +    '''
>> +
>> +    def run(self):
>> +        realcopyfile = file_util.copy_file
>> +        def copyfileandsetmode(*args, **kwargs):
>> +            src, dst = args[0], args[1]
>> +            dst, copied = realcopyfile(*args, **kwargs)
>> +            if copied:
>> +                st = os.stat(src)
>> +                # Persist executable bit (apply it to group and other if
>> user
>> +                # has it)
>> +                if st[stat.ST_MODE] & stat.S_IXUSR:
>> +                    setmode = 0755
>> +                else:
>> +                    setmode = 0644
>> +                os.chmod(dst, (stat.S_IMODE(st[stat.ST_MODE]) & ~0777) |
>> +                         setmode)
>> +        file_util.copy_file = copyfileandsetmode
>> +        try:
>> +            install_lib.run(self)
>> +        finally:
>> +            file_util.copy_file = realcopyfile
>> +
>>
>>
>


-- 
--Kyle

Note:
If I've asked a question, and you're responding to me, please use *respond
all*, so that other people can read any solutions we come to!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20141001/8b659b58/attachment.html>


More information about the Mercurial-devel mailing list