<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 1, 2014 at 5:40 PM, Mads Kiilerich <span dir="ltr"><<a href="mailto:mads@kiilerich.com" target="_blank" class="cremed">mads@kiilerich.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 10/01/2014 11:00 PM, Kyle Lippincott wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
# HG changeset patch<br>
# User Kyle Lippincott <<a href="mailto:spectral@google.com" target="_blank" class="cremed">spectral@google.com</a>><br>
# Date 1412122434 25200<br>
#      Tue Sep 30 17:13:54 2014 -0700<br>
# Node ID b168f18ef708e5c834eb5ce9f6804c<u></u>033e769082<br>
# Parent  939ce500c92a3dcc0e10815242361f<u></u>f70a6fcae9<br>
setup: Set mode 644 or 755 on installed files<br>
</blockquote>
<br></span>
The Scripture says that the subject line should be all lower case.<br></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
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.<br></blockquote><div><br></div><div>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?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
It is sad that such a minor change requires so much code. But distutils :-(<br></blockquote><div><br></div><div>Yeah.  :(</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Couldn't we just set umask in this script or in Makefile ... or fail early if it is bad?</blockquote><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="HOEnZb"><font color="#888888"><br>
<br>
/Mads</font></span><span class="im HOEnZb"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
diff -r 939ce500c92a -r b168f18ef708 setup.py<br>
--- a/setup.py  Mon Sep 29 17:23:38 2014 -0500<br>
+++ b/setup.py  Tue Sep 30 17:13:54 2014 -0700<br>
</blockquote>
<br>
</span><div class="HOEnZb"><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
@@ -375,6 +376,39 @@<br>
                                        libraries=[],<br>
                                        output_dir=self.build_temp)<br>
  +class hginstalllib(install_lib):<br>
+    '''<br>
+    This is a specialization of install_lib that replaces the copy_file used<br>
+    there so that it supports setting the mode of files after copying them,<br>
+    instead of just preserving the mode that the files originally had.  If your<br>
+    system has a umask of something like 027, preserving the permissions when<br>
+    copying will lead to a broken install.<br>
+<br>
+    Note that just passing keep_permissions=False to copy_file would be<br>
+    insufficient, as it might still be applying a umask.<br>
+    '''<br>
+<br>
+    def run(self):<br>
+        realcopyfile = file_util.copy_file<br>
+        def copyfileandsetmode(*args, **kwargs):<br>
+            src, dst = args[0], args[1]<br>
+            dst, copied = realcopyfile(*args, **kwargs)<br>
+            if copied:<br>
+                st = os.stat(src)<br>
+                # Persist executable bit (apply it to group and other if user<br>
+                # has it)<br>
+                if st[stat.ST_MODE] & stat.S_IXUSR:<br>
+                    setmode = 0755<br>
+                else:<br>
+                    setmode = 0644<br>
+                os.chmod(dst, (stat.S_IMODE(st[stat.ST_MODE]<u></u>) & ~0777) |<br>
+                         setmode)<br>
+        file_util.copy_file = copyfileandsetmode<br>
+        try:<br>
+            install_lib.run(self)<br>
+        finally:<br>
+            file_util.copy_file = realcopyfile<br>
+<br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>--Kyle</div><div><br></div><div>Note:</div>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!<br><div><br></div>
</div></div>