Patchwork D1580: setup: only write some autogenerated files if they change

login
register
mail settings
Submitter phabricator
Date Dec. 4, 2017, 5 a.m.
Message ID <differential-rev-PHID-DREV-n3yhwtiqbaqmxsyuwjx3-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/25911/
State Superseded
Headers show

Comments

phabricator - Dec. 4, 2017, 5 a.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Without this change, setup.py always writes some files on every
  invocation. This prevents some builds from being a no-op when they
  should. And, since times can sneak into generated .pyc files,
  this prevents file content from being deterministic between
  builds.
  
  As part of the refactor, we treat file content as bytes.
  
  The only potential regression from this would be if some tool
  is looking at mtimes of the changed files to determine if
  further action should be taken. But I don't think anything
  critically important is keyed off the mtimes of these specific files.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1580

AFFECTED FILES
  setup.py

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 5, 2017, 11:56 a.m.
yuja accepted this revision.
yuja added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> setup.py:499
> +
> +        content = ''.join([
> +            '# this file is autogenerated by setup.py\n',

Made it bytes and queued, thanks.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1580

To: indygreg, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -136,6 +136,18 @@ 
 from distutils.sysconfig import get_python_inc, get_config_var
 from distutils.version import StrictVersion
 
+def write_if_changed(path, content):
+    """Write content to a file iff the content hasn't changed."""
+    if os.path.exists(path):
+        with open(path, 'rb') as fh:
+            current = fh.read()
+    else:
+        current = ''
+
+    if current != content:
+        with open(path, 'wb') as fh:
+            fh.write(content)
+
 scripts = ['hg']
 if os.name == 'nt':
     # We remove hg.bat if we are able to build hg.exe.
@@ -317,9 +329,14 @@ 
         version = kw.get('node', '')[:12]
 
 if version:
-    with open("mercurial/__version__.py", "w") as f:
-        f.write('# this file is autogenerated by setup.py\n')
-        f.write('version = "%s"\n' % version)
+    versionb = version
+    if not isinstance(versionb, bytes):
+        versionb = versionb.encode('ascii')
+
+    write_if_changed('mercurial/__version__.py', b''.join([
+        b'# this file is autogenerated by setup.py\n'
+        b'version = "%s"\n' % versionb,
+    ]))
 
 try:
     oldpolicy = os.environ.get('HGMODULEPOLICY', None)
@@ -478,9 +495,13 @@ 
             modulepolicy = 'allow'
         else:
             modulepolicy = 'c'
-        with open(os.path.join(basepath, '__modulepolicy__.py'), "w") as f:
-            f.write('# this file is autogenerated by setup.py\n')
-            f.write('modulepolicy = b"%s"\n' % modulepolicy)
+
+        content = ''.join([
+            '# this file is autogenerated by setup.py\n',
+            'modulepolicy = b"%s"\n' % modulepolicy,
+        ])
+        write_if_changed(os.path.join(basepath, '__modulepolicy__.py'),
+                         content)
 
         build_py.run(self)