Re: [PATCH] D24470: [analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

[Devin Coughlin via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281516: [analyzer] scan-build-py: Remove relative path hack 
for SATestsBuild.py (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D24470?vs=71046=71401#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24470

Files:
  cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
  cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py

Index: cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
===
--- cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
+++ cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
===
--- cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
+++ cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 


Index: cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
===
--- cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
+++ cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
===
--- cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
+++ cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24470: [analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

[Laszlo Nagy via cfe-commits]

rizsotto.mailinglist accepted this revision.
rizsotto.mailinglist added a comment.
This revision is now accepted and ready to land.

thanks Devin, i like smaller code, have no problem with this change. :)

about the file/directory paths: i agree that the situation is not ideal. but 
there are/were strict constrains on how a compilation database shall look like. 
once a year i ask the question 
 on the 
mailing list about it. the situation might have been improved since... besides, 
using absolute path is actually the safest bet we can make. (other tools doing 
the same (CMake is a good example)) would not invest time into this until i see 
it got broken.


https://reviews.llvm.org/D24470



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24470: [analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

[Devin Coughlin via cfe-commits]

dcoughlin created this revision.
dcoughlin added a reviewer: rizsotto.mailinglist.
dcoughlin added a subscriber: cfe-commits.

Remove the relative path hack in scan-build-py that converts a fully qualified 
directory name and a fully qualified file path to a relative path before 
running the analyzer on a file.

This hack is not needed: the bad interaction with SATestsBuild.py it was 
intended to address is actually the same underlying problem that D24163 fixed. 
Further, because the hack would always relativize paths, it caused 
SATestBuild.py to be unable to properly line up issues when the build system 
changed directory and then built a source file in a child directory but used a 
fully-qualified path for the source file.

Laszlo: Are you OK with me removing this hack? This is the last outstanding 
issue in scan-build-py preventing us in using it for our regression suite.

Note: I still think the approach used here (and especially in intercept-build) 
is not ideal. I really think we should be storing both the working directory 
and the actual path argument (even if it is relative) in the compilation 
database. Storing the fully-qualified paths loses information (i.e., which of 
the many possible paths for that file was actually used) and makes replaying 
the build with fidelity impossible. That said, after D24163 I don't have any 
examples where is actually an issue in practice.

https://reviews.llvm.org/D24470

Files:
  tools/scan-build-py/libscanbuild/runner.py
  tools/scan-build-py/tests/unit/test_runner.py

Index: tools/scan-build-py/tests/unit/test_runner.py
===
--- tools/scan-build-py/tests/unit/test_runner.py
+++ tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: tools/scan-build-py/libscanbuild/runner.py
===
--- tools/scan-build-py/libscanbuild/runner.py
+++ tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 


Index: tools/scan-build-py/tests/unit/test_runner.py
===
--- tools/scan-build-py/tests/unit/test_runner.py
+++ tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: tools/scan-build-py/libscanbuild/runner.py
===
--- tools/scan-build-py/libscanbuild/runner.py
+++ tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def