labath added a comment.
Davide, I reverted this because it breaks the case when you just specify a
filename as a compiler (with the expectation that we will look it up in the
path). I think this is a good thing to have, and our buildbot was using that
behavior.
I like the direction this change
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316393: [lldbtests] Handle errors instead of crashing.
(authored by davide).
Changed prior to commit:
https://reviews.llvm.org/D39199?vs=119917&id=119965#toc
Repository:
rL LLVM
https://reviews.llvm
jasonmolenda added a comment.
I think this is a good idea, thanks for writing the patch Davide.
https://reviews.llvm.org/D39199
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a comment.
In https://reviews.llvm.org/D39199#904169, @zturner wrote:
> I just wanted to make sure nobody was *already* relying on that behavior. If
> we can get away with being strict, we should be strict.
I agree. From a quick skim I don't see anything really relying on that, b
zturner added a comment.
I just wanted to make sure nobody was *already* relying on that behavior. If
we can get away with being strict, we should be strict.
https://reviews.llvm.org/D39199
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
davide added inline comments.
Comment at: packages/Python/lldbsuite/test/dotest.py:52
def is_exe(fpath):
+"""Returns true if fpath is an executable.
clayborg wrote:
> We could add a default parameter here like:
>
> ```
> def is_exe(fpath, fatal=False):
>
clayborg added inline comments.
Comment at: packages/Python/lldbsuite/test/dotest.py:52
def is_exe(fpath):
+"""Returns true if fpath is an executable.
We could add a default parameter here like:
```
def is_exe(fpath, fatal=False):
if fatal and not os.pa
davide added a comment.
If there are cases where `is_exe` shouldn't be fatal, then we might consider
splitting the utility into two bits (exists & is_exe).
In my mind, if you're checking whether something is executable you should run
the check on a valid entity, and it's up to the caller guarant
davide added a comment.
Apologies, I generally do (but I forgot this time), let me update the patch.
Repository:
rL LLVM
https://reviews.llvm.org/D39199
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
davide updated this revision to Diff 119917.
https://reviews.llvm.org/D39199
Files:
packages/Python/lldbsuite/test/dotest.py
Index: packages/Python/lldbsuite/test/dotest.py
===
--- packages/Python/lldbsuite/test/dotest.py
+++ pac
zturner added a comment.
Can you upload diffs with context in the future?
I'm trying to figure out whether `is_exe` is used for anything where
non-existance should not be considered fatal, but I can't see the rest of the
file here.
Repository:
rL LLVM
https://reviews.llvm.org/D39199
___
davide added inline comments.
Comment at: packages/Python/lldbsuite/test/dotest.py:56
+sys.exit(-1)
return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
lemo wrote:
> minor: can we print quotes around fpath? (this usually helps in messages to
lemo added inline comments.
Comment at: packages/Python/lldbsuite/test/dotest.py:56
+sys.exit(-1)
return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
minor: can we print quotes around fpath? (this usually helps in messages to
avoid confusion
davide created this revision.
If you pass an invalid compiler/debugger path on the cmdline to `dotest.py`
this is what you get.
$ python dotest.py --executable /home/davide/work/build-lldb/bin/lldb
--compiler /home/davide/work/build-lldb/bin/clandasfaasdfsg
Traceback (most recent call last
14 matches
Mail list logo
