[clang-tidy] run-clang-tidy fails using python 3.7

[Abpostelnicu]

Summary

Because the data types like str and byte changed in python 3.x we get this error when running run-clang-tidy:

Exception in thread Thread-1:
Traceback (most recent call last):

File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/threading.py", line 917, in _bootstrap_inner
  self.run()
File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/threading.py", line 865, in run
  self._target(*self._args, **self._kwargs)
File "run-clang-tidy.py", line 178, in run_tidy
  sys.stdout.write(' '.join(invocation) + '\n' + output + '\n')

TypeError: can only concatenate str (not "bytes") to str

[JonasToth]

Please create the patch with full context (https://www.llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)

[JonasToth]

I don't know a lot about how to write portable code!

But wouldn't this be out usecase? http://python-future.org/compatible_idioms.html#file-io-with-open
Using the decode is supposed to work in both pythons

[Abpostelnicu]

In D51220#1212463, @JonasToth wrote:

I don't know a lot about how to write portable code!

But wouldn't this be out usecase? http://python-future.org/compatible_idioms.html#file-io-with-open
Using the decode is supposed to work in both pythons

Yes, indeed, this is more elegant solution!

[JonasToth]

Could you please verify it is actually working?

Am 24.08.2018 um 18:05 schrieb Andi via Phabricator:

Abpostelnicu added a comment.

In https://reviews.llvm.org/D51220#1212463, @JonasToth wrote:

I don't know a lot about how to write portable code!

But wouldn't this be out usecase? http://python-future.org/compatible_idioms.html#file-io-with-open
Using the decode is supposed to work in both pythons

Yes, indeed, this is more elegant solution!

https://reviews.llvm.org/D51220

[Abpostelnicu]

I can confirm tested on:
2.7.15
3.7.0

On both it worked.

• signature.asc848 BDownload

[JonasToth]

LG from my side, but please let @aaron.ballman or @alexfh approve first

Am 24.08.2018 um 18:08 schrieb Andi via Phabricator:

Abpostelnicu added a comment.

I can confirm tested on:
2.7.15
3.7.0

On both it worked.

• F7047650: signature.asc https://reviews.llvm.org/F7047650

https://reviews.llvm.org/D51220

[lebedev.ri]

Have you seen D36624 / D38289 ?
Please test how it behaves with both the python2 and python3 when the clang-tidy output contains non-ASCII symbols.

[JonasToth]

Unicode works both with python3 and python2 (but seemed to work before too).

Otherwise LG

clang-tidy/tool/run-clang-tidy.py

169

The patch does not apply clean to master because of this part. Could you please recheck that?

171

please use len(err) here instead, python3 does not work.

clang-tidy/tool/run-clang-tidy.py

invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
                                      tmpdir, build_path, args.header_filter,
                                      args.extra_arg, args.extra_arg_before,
                                      args.quiet, args.config)
 
     proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     output, err = proc.communicate()
     if proc.returncode != 0:
       failed_files.append(name)
-
-    if is_py2:
-      output_string = output
-      err_string = err
-    else:
-      output_string = str(output, 'utf-8')
-      err_string = str(err, 'utf-8')
-
     with lock:
-      sys.stdout.write(' '.join(invocation) + '\n' + output_string + '\n')
+      sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + '\n')
       if err:
-        sys.stderr.write(err_string + '\n')
+        sys.stderr.write(err.decode('utf-8') + '\n')
     queue.task_done()
 
 
 def main():
   parser = argparse.ArgumentParser(description='Runs clang-tidy over all files '
                                    'in a compilation database. Requires '
                                    'clang-tidy and clang-apply-replacements in '
                                    '$PATH.')