[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Tim Shen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316411: [test] Fix clang-test for FreeBSD and NetBSD 
(authored by timshen).

Changed prior to commit:
  https://reviews.llvm.org/D39162?vs=119803=119995#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39162

Files:
  cfe/trunk/test/Unit/lit.cfg.py


Index: cfe/trunk/test/Unit/lit.cfg.py
===
--- cfe/trunk/test/Unit/lit.cfg.py
+++ cfe/trunk/test/Unit/lit.cfg.py
@@ -35,17 +35,23 @@
 if symbolizer in os.environ:
 config.environment[symbolizer] = os.environ[symbolizer]
 
-shlibpath_var = ''
-if platform.system() == 'Linux':
-shlibpath_var = 'LD_LIBRARY_PATH'
-elif platform.system() == 'Darwin':
-shlibpath_var = 'DYLD_LIBRARY_PATH'
-elif platform.system() == 'Windows':
-shlibpath_var = 'PATH'
-
-# in stand-alone builds, shlibdir is clang's build tree
-# while llvm_libs_dir is installed LLVM (and possibly older clang)
-shlibpath = os.path.pathsep.join((config.shlibdir, config.llvm_libs_dir,
- config.environment.get(shlibpath_var,'')))
-
-config.environment[shlibpath_var] = shlibpath
+def find_shlibpath_var():
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
+yield 'LD_LIBRARY_PATH'
+elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+# in stand-alone builds, shlibdir is clang's build tree
+# while llvm_libs_dir is installed LLVM (and possibly older clang)
+shlibpath = os.path.pathsep.join(
+(config.shlibdir,
+ config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+config.environment[shlibpath_var] = shlibpath
+break
+else:
+lit_config.warning("unable to inject shared library path on '{}'"
+   .format(platform.system()))


Index: cfe/trunk/test/Unit/lit.cfg.py
===
--- cfe/trunk/test/Unit/lit.cfg.py
+++ cfe/trunk/test/Unit/lit.cfg.py
@@ -35,17 +35,23 @@
 if symbolizer in os.environ:
 config.environment[symbolizer] = os.environ[symbolizer]
 
-shlibpath_var = ''
-if platform.system() == 'Linux':
-shlibpath_var = 'LD_LIBRARY_PATH'
-elif platform.system() == 'Darwin':
-shlibpath_var = 'DYLD_LIBRARY_PATH'
-elif platform.system() == 'Windows':
-shlibpath_var = 'PATH'
-
-# in stand-alone builds, shlibdir is clang's build tree
-# while llvm_libs_dir is installed LLVM (and possibly older clang)
-shlibpath = os.path.pathsep.join((config.shlibdir, config.llvm_libs_dir,
- config.environment.get(shlibpath_var,'')))
-
-config.environment[shlibpath_var] = shlibpath
+def find_shlibpath_var():
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
+yield 'LD_LIBRARY_PATH'
+elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+# in stand-alone builds, shlibdir is clang's build tree
+# while llvm_libs_dir is installed LLVM (and possibly older clang)
+shlibpath = os.path.pathsep.join(
+(config.shlibdir,
+ config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+config.environment[shlibpath_var] = shlibpath
+break
+else:
+lit_config.warning("unable to inject shared library path on '{}'"
+   .format(platform.system()))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added a comment.

ping?


https://reviews.llvm.org/D39162



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

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 119803.
lichray edited the summary of this revision.
lichray added a comment.

Changed to an warning given @zturner 's comments and experiments.


https://reviews.llvm.org/D39162

Files:
  test/Unit/lit.cfg.py


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -35,17 +35,23 @@
 if symbolizer in os.environ:
 config.environment[symbolizer] = os.environ[symbolizer]
 
-shlibpath_var = ''
-if platform.system() == 'Linux':
-shlibpath_var = 'LD_LIBRARY_PATH'
-elif platform.system() == 'Darwin':
-shlibpath_var = 'DYLD_LIBRARY_PATH'
-elif platform.system() == 'Windows':
-shlibpath_var = 'PATH'
-
-# in stand-alone builds, shlibdir is clang's build tree
-# while llvm_libs_dir is installed LLVM (and possibly older clang)
-shlibpath = os.path.pathsep.join((config.shlibdir, config.llvm_libs_dir,
- config.environment.get(shlibpath_var,'')))
-
-config.environment[shlibpath_var] = shlibpath
+def find_shlibpath_var():
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
+yield 'LD_LIBRARY_PATH'
+elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+# in stand-alone builds, shlibdir is clang's build tree
+# while llvm_libs_dir is installed LLVM (and possibly older clang)
+shlibpath = os.path.pathsep.join(
+(config.shlibdir,
+ config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+config.environment[shlibpath_var] = shlibpath
+break
+else:
+lit_config.warning("unable to inject shared library path on '{}'"
+   .format(platform.system()))


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -35,17 +35,23 @@
 if symbolizer in os.environ:
 config.environment[symbolizer] = os.environ[symbolizer]
 
-shlibpath_var = ''
-if platform.system() == 'Linux':
-shlibpath_var = 'LD_LIBRARY_PATH'
-elif platform.system() == 'Darwin':
-shlibpath_var = 'DYLD_LIBRARY_PATH'
-elif platform.system() == 'Windows':
-shlibpath_var = 'PATH'
-
-# in stand-alone builds, shlibdir is clang's build tree
-# while llvm_libs_dir is installed LLVM (and possibly older clang)
-shlibpath = os.path.pathsep.join((config.shlibdir, config.llvm_libs_dir,
- config.environment.get(shlibpath_var,'')))
-
-config.environment[shlibpath_var] = shlibpath
+def find_shlibpath_var():
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
+yield 'LD_LIBRARY_PATH'
+elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+# in stand-alone builds, shlibdir is clang's build tree
+# while llvm_libs_dir is installed LLVM (and possibly older clang)
+shlibpath = os.path.pathsep.join(
+(config.shlibdir,
+ config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+config.environment[shlibpath_var] = shlibpath
+break
+else:
+lit_config.warning("unable to inject shared library path on '{}'"
+   .format(platform.system()))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Zachary Turner via cfe-commits]

Because this is not tge only part of ghe library search algorithm, and it’s
not necessarily an error if the variable isn’t set

On Sun, Oct 22, 2017 at 1:56 PM Zhihao Yuan via Phabricator <
revi...@reviews.llvm.org> wrote:

> lichray marked an inline comment as done.
> lichray added a comment.
>
> In https://reviews.llvm.org/D39162#903179, @zturner wrote:
>
> > Please don't throw an exception here.  Instead, write this as:
> >
> >   lit_config.warning('Unable to determine shared library path variable
> for platform {}'.format(platform.system()))
> >
> >
>
>
> Why a warning rather than fatal?
>
>
> https://reviews.llvm.org/D39162
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In https://reviews.llvm.org/D39162#903190, @krytarowski wrote:

> In https://reviews.llvm.org/D39162#903186, @joerg wrote:
>
> > I think we should special case Darwin and Windows and fall-back to 
> > LD_LIBRARY_PATH for the rest. Can't remember if there is a UNIX-like 
> > platform left where it doesn't work.
>
>
> SHLIB_PATH in HP/UX


Hmm SHLIB_PATH has been marked as legacy and they probably switched to 
LD_LIBRARY_PATH.

AIX might still ship with LIBPATH.

I agree that adding defines for OS'es that are barely developed (especially 
closed-source) without LLVM developers might be accumulating unused code.


https://reviews.llvm.org/D39162



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

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 119800.

https://reviews.llvm.org/D39162

Files:
  test/Unit/lit.cfg.py


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -35,13 +35,15 @@
 if symbolizer in os.environ:
 config.environment[symbolizer] = os.environ[symbolizer]
 
-shlibpath_var = ''
-if platform.system() == 'Linux':
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
 shlibpath_var = 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 shlibpath_var = 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 shlibpath_var = 'PATH'
+else:
+lit_config.fatal("unable to inject shared library path on '{}'"
+ .format(platform.system()))
 
 # in stand-alone builds, shlibdir is clang's build tree
 # while llvm_libs_dir is installed LLVM (and possibly older clang)


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -35,13 +35,15 @@
 if symbolizer in os.environ:
 config.environment[symbolizer] = os.environ[symbolizer]
 
-shlibpath_var = ''
-if platform.system() == 'Linux':
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
 shlibpath_var = 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 shlibpath_var = 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 shlibpath_var = 'PATH'
+else:
+lit_config.fatal("unable to inject shared library path on '{}'"
+ .format(platform.system()))
 
 # in stand-alone builds, shlibdir is clang's build tree
 # while llvm_libs_dir is installed LLVM (and possibly older clang)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In https://reviews.llvm.org/D39162#903186, @joerg wrote:

> I think we should special case Darwin and Windows and fall-back to 
> LD_LIBRARY_PATH for the rest. Can't remember if there is a UNIX-like platform 
> left where it doesn't work.


SHLIB_PATH in HP/UX


https://reviews.llvm.org/D39162



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

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added a comment.

In https://reviews.llvm.org/D39162#903186, @joerg wrote:

> I think we should special case Darwin and Windows and fall-back to 
> LD_LIBRARY_PATH for the rest. Can't remember if there is a UNIX-like platform 
> left where it doesn't work.


If those developers come to us, I would say.  The script is for LLVM developers 
only, and to developers, a missed guess wastes more time than pointing out 
where it doesn't work.


https://reviews.llvm.org/D39162



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

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I think we should special case Darwin and Windows and fall-back to 
LD_LIBRARY_PATH for the rest. Can't remember if there is a UNIX-like platform 
left where it doesn't work.


https://reviews.llvm.org/D39162



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

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Zhihao Yuan via Phabricator via cfe-commits]

lichray marked an inline comment as done.
lichray added a comment.

In https://reviews.llvm.org/D39162#903179, @zturner wrote:

> Please don't throw an exception here.  Instead, write this as:
>
>   lit_config.warning('Unable to determine shared library path variable for 
> platform {}'.format(platform.system()))
>
>  


Why a warning rather than fatal?


https://reviews.llvm.org/D39162



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

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Zachary Turner via Phabricator via cfe-commits]

zturner requested changes to this revision.
zturner added a comment.
This revision now requires changes to proceed.

Please don't throw an exception here.  Instead, write this as:

  shlibpath_var = None
  if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
  shilbpath = 'LD_LIBRARY_PATH'
  elif platform.system() == 'Darwin':
  shlibpath_var = 'DYLD_LIBRARY_PATH'
  elif platform.system() == 'Windows':
  shlibpath_var = 'PATH'
  
  if shlibpath_var:
  shlibpath = os.path.pathsep.join((config.shlibdir, config.llvm_libs_dir, 
config.environment.get(shlibpath_var,'')))
  config.environment[shlibpath_var] = shlibpath
  else:
  lit_config.warning('Unable to determine shared library path variable for 
platform {}'.format(platform.system()))


https://reviews.llvm.org/D39162



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

[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 119772.
lichray retitled this revision from "[test] Fix clang-test for FreeBSD" to 
"[test] Fix clang-test for FreeBSD and NetBSD".
lichray edited the summary of this revision.

https://reviews.llvm.org/D39162

Files:
  test/Unit/lit.cfg.py


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -36,12 +36,14 @@
 config.environment[symbolizer] = os.environ[symbolizer]
 
 shlibpath_var = ''
-if platform.system() == 'Linux':
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
 shlibpath_var = 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 shlibpath_var = 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 shlibpath_var = 'PATH'
+else:
+raise EnvironmentError('unknown platform.system()')
 
 # in stand-alone builds, shlibdir is clang's build tree
 # while llvm_libs_dir is installed LLVM (and possibly older clang)


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -36,12 +36,14 @@
 config.environment[symbolizer] = os.environ[symbolizer]
 
 shlibpath_var = ''
-if platform.system() == 'Linux':
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
 shlibpath_var = 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 shlibpath_var = 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 shlibpath_var = 'PATH'
+else:
+raise EnvironmentError('unknown platform.system()')
 
 # in stand-alone builds, shlibdir is clang's build tree
 # while llvm_libs_dir is installed LLVM (and possibly older clang)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits