[gn build] Fix Python DeprecationWarning

[mantognini]

Summary

This fixes two issues:

• DeprecationWarning: invalid escape sequence \`
• ResourceWarning: unclosed file

[mantognini]

Let me know what you think of this patch. Thanks.

[thakis]

llvm/utils/gn/build/write_cmake_config.py

65

What warns about this? Python itself?

The garbage collector cleans up the file, and this is a very short-lived script anyways, so I'd prefer brevity over linter happyiness if this is to placate some linter.

If python3 warns about this by default, I suppose it's fine to fix this.

[mantognini]

llvm/utils/gn/build/write_cmake_config.py

65

Yep, starting with Python 3.7.4 and PEP565, those warnings are emitted by default:

To improve this situation, this PEP proposes a single adjustment to the default warnings filter: displaying deprecation warnings attributed to the main module by default.

This is not an extra linter that warns about this, but Python itself.

I'd further recommend fixing this to reduce the noise in our downstream tests.

[thakis]

Hm, I'm trying to reproduce the warnings locally by adding script_executable = "python3" to llvm/utils/gn/.gn and doing a full build. Everything looks happy. I checked that out/gn/toolchain.ninja does indeed invoke python3 for python actions. What am I doing wrong? How do I reproduce these warnings?

[mantognini]

You need Python 3.7.4 or later. The warnings are emitted only starting from this minor release.

[thakis]

I'm on exactly 3.7.4:

$ python3 --version
Python 3.7.4

Can you paste the commands you run locally to see the warnings?

[mantognini]

I'm afraid I can't give you the exact log we have, because we have a special integration of LLVM downstream and don't directly rely on the usual cmake/gn/... workflow. I'm sorry about that.

What I can tell you is that we invoke this script as a standalone program, i.e. we run ./some/path/write_cmake_config.py <args>. This means the she-bang line is being used, and python (not python3) is being used. I don't know exactly how this script is invoked when using the "regular" build-flow, but could it be that the python binary is actually Python 2?

[thakis]

What's the output of /usr/bin/env python --version on your system?

[mantognini]

/usr/bin/env python --version
Python 3.7.0

Apparently I was slightly wrong: it's not specific to the .4 maintenance release but any 3.7 version. On the download page I though the items applied to 3.7.4 only, but as can be confirmed by the 3.7 documentation page it's not. Apologies for this. The fix is still relevant though.

I assume you couldn't reproduce the warnings. Do you have by any change some PYTHONWARNINGS environment variable turning the warning off or some command line argument that does the same?

[thakis]

Apparently I was slightly wrong: it's not specific to the .4 maintenance release but any 3.7 version. On the download page I though the items applied to 3.7.4 only, but as can be confirmed by the 3.7 documentation page it's not. Apologies for this. The fix is still relevant though.

I assume you couldn't reproduce the warnings. Do you have by any change some PYTHONWARNINGS environment variable turning the warning off or some command line argument that does the same?

Aha, thanks, if I run PYTHONWARNINGS=default ninja -C out/gn -j200 check-clang I see the warnings. But by default they're not on for me, and I don't have PYTHONWARNINGS set. Do you set it to anything? Does anything else set it in your env?

[mantognini]

I don't have PYTHONWARNINGS set in my env. Maybe ninja invoke the script using python -Wignore write_cmake_config.py or something? It's quite worrisome that Python runtime can exhibit different behaviours without apparent reasons. :/

[thakis]

In D67753#1680892, @mantognini wrote:

I don't have PYTHONWARNINGS set in my env. Maybe ninja invoke the script using python -Wignore write_cmake_config.py or something? It's quite worrisome that Python runtime can exhibit different behaviours without apparent reasons. :/

That's mysterious indeed. Looks like the default warnings come from here: https://github.com/python/cpython/blob/88e6447451fb5525e83e802c66c3e51b4a45bf86/Python/initconfig.c#L2064 You don't happen to run python in dev mode, do you? In dev mode, the warnings default to "default"; else they apparently don't.

[mantognini]

In D67753#1681049, @thakis wrote:

That's mysterious indeed. Looks like the default warnings come from here: https://github.com/python/cpython/blob/88e6447451fb5525e83e802c66c3e51b4a45bf86/Python/initconfig.c#L2064 You don't happen to run python in dev mode, do you? In dev mode, the warnings default to "default"; else they apparently don't.

I think I found out why I get those warnings: the Python binary we use for our test was built with --with-pydebug. I've tested this by downloading Python source and building it with and without that ./configure option and it seems to change the behaviour of Python runtime. Using GDB, I inspected config_init_warnoptions. The options are set apparently in the same way. Turns out, the difference are in init_filters, which turns all warnings on when built using --with-pydebug but only the ones mentioned in the documentation when built without this option, and in get_filter, which uses get_default_action under --with-pydebug.

Strictly speaking, this means that the warnings we see downstream are not triggered by default with Python 3.7. I still think it's worth fixing these, though.

[thakis]

Strictly speaking, this means that the warnings we see downstream are not triggered by default with Python 3.7. I still think it's worth fixing these, though.

Meh. The backslash escape fix maybe, but the files are freed at script end anyways, so it's a bunch of lines more for close to 0 benefit ¯\_(ツ)_/¯

[thakis]

…but thanks for getting to the bottom of why you're seeing the warnings, that's good to know :)

And given that this is committed already, it's fine to keep it in.

llvm/utils/gn/build/write_cmake_config.py

 #!/usr/bin/env python
-"""Emulates the bits of CMake's configure_file() function needed in LLVM.
+r"""Emulates the bits of CMake's configure_file() function needed in LLVM.
 
 The CMake build uses configure_file() for several things.  This emulates that
 function for the GN build.  In the GN build, this runs at build time instead
 of at generator time.
 
 Takes a list of KEY=VALUE pairs (where VALUE can be empty).
 
 The sequence `\` `n` in each VALUE is replaced by a newline character.
for value in args.values:
             print('duplicate key "%s" in args' % key, file=sys.stderr)
             return 1
         values[key] = val.replace('\\n', '\n')
     unused_values = set(values.keys())
 
     # Matches e.g. '${FOO}' or '@FOO@' and captures FOO in group 1 or 2.
     var_re = re.compile(r'\$\{([^}]*)\}|@([^@]*)@')
 
-    in_lines = open(args.input).readlines()
+    with open(args.input) as f:
+        in_lines = f.readlines()
     out_lines = []
     for in_line in in_lines:
         def repl(m):
             key = m.group(1) or m.group(2)
             unused_values.discard(key)
             return values[key]
         in_line = var_re.sub(repl, in_line)
         if in_line.startswith('#cmakedefine01 '):
def main():
     leftovers = var_re.findall(output)
     if leftovers:
         print(
             'unprocessed values:\n',
             '\n'.join([x[0] or x[1] for x in leftovers]),
             file=sys.stderr)
         return 1
 
-    if not os.path.exists(args.output) or open(args.output).read() != output:
-        open(args.output, 'w').write(output)
+    def read(filename):
+        with open(args.output) as f:
+            return f.read()
+
+    if not os.path.exists(args.output) or read(args.output) != output:
+        with open(args.output, 'w') as f:
+            f.write(output)
         os.chmod(args.output, os.stat(args.input).st_mode & 0o777)
 
 
 if __name__ == '__main__':
     sys.exit(main())