This revision was automatically updated to reflect the committed changes.
Closed by commit rL289956: Fix broken escaping of commands in the build
(authored by ldrumm).
Changed prior to commit:
https://reviews.llvm.org/D26757?vs=79578&id=81761#toc
Repository:
rL LLVM
https://reviews.llvm.org
Hi Zachary
On 15/12/16 17:52, Zachary Turner wrote:
I think this was lgtm'ed wasn't it?
Chris checked the cmake changes, so if you're happy with the python
fixes I'll go ahead and commit this.
Thanks
Luke
___
lldb-commits mailing list
lldb-commit
I think this was lgtm'ed wasn't it?
On Thu, Dec 15, 2016 at 9:46 AM Luke Drummond via Phabricator <
revi...@reviews.llvm.org> wrote:
> ldrumm marked 2 inline comments as done.
> ldrumm added a comment.
>
> ping
>
>
> https://reviews.llvm.org/D26757
>
>
>
>
ldrumm marked 2 inline comments as done.
ldrumm added a comment.
ping
https://reviews.llvm.org/D26757
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
bryant added inline comments.
Comment at: CMakeLists.txt:53
+ --lldbLibDir=lib${LLVM_LIBDIR_SUFFIX}
+ ${FINISH_EXTRA_ARGS}
+VERBATIM
The indentation here could match the above.
https://reviews.llvm.org/D26757
_
bryant added inline comments.
Comment at: scripts/Python/prepare_binding_Python.py:222
+)
+logging.info("running swig with: %r", command)
ldrumm wrote:
> bryant wrote:
> > You can reduce diff noise by limiting your changes to removing the %s. So,
> >
>
bryant added inline comments.
Comment at: CMakeLists.txt:46
+COMMAND
+ ${PYTHON_EXECUTABLE}
${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
+ --srcRoot=${LLDB_SOURCE_DIR}
You can reduce diff noise by leaving format
granata.enrico resigned from this revision.
granata.enrico removed a reviewer: granata.enrico.
granata.enrico added a comment.
I am not an Apple employee working on LLDB anymore
https://reviews.llvm.org/D26757
___
lldb-commits mailing list
lldb-comm
ldrumm added a subscriber: lldb-commits.
ldrumm updated this revision to Diff 79578.
ldrumm added a comment.
Addressed some unwanted whitespace changes as suggested by @bryant; use
`os.dir.curdir` as instead of `.`
https://reviews.llvm.org/D26757
Files:
CMakeLists.txt
scripts/CMakeLists.tx
beanz added a comment.
The CMake all LGTM.
https://reviews.llvm.org/D26757
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
ldrumm added inline comments.
Comment at: scripts/Python/prepare_binding_Python.py:222
+)
+logging.info("running swig with: %r", command)
bryant wrote:
> ldrumm wrote:
> > bryant wrote:
> > > You can reduce diff noise by limiting your changes to removin
ldrumm added inline comments.
Comment at: scripts/CMakeLists.txt:38
+ COMMAND
+${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/prepare_bindings.py
+ ${framework_arg}
bryant wrote:
> Move this back up.
Will do
https://reviews.llvm.org/D26757
__
ldrumm added inline comments.
Comment at: scripts/Python/prepare_binding_Python.py:222
+)
+logging.info("running swig with: %r", command)
bryant wrote:
> You can reduce diff noise by limiting your changes to removing the %s. So,
>
> ```python
> # B
ldrumm added inline comments.
Comment at: CMakeLists.txt:46
+COMMAND
+ ${PYTHON_EXECUTABLE}
${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
+ --srcRoot=${LLDB_SOURCE_DIR}
bryant wrote:
> You can reduce diff noise b
ldrumm added inline comments.
Comment at: scripts/Python/prepare_binding_Python.py:229
stderr=subprocess.PIPE,
-shell=True)
+)
# Wait for SWIG process to terminate
bryant wrote:
> ldrumm wrote:
> > granata.enrico wrote:
> > > This worrie
ldrumm added inline comments.
Comment at: scripts/Python/prepare_binding_Python.py:229
stderr=subprocess.PIPE,
-shell=True)
+)
# Wait for SWIG process to terminate
granata.enrico wrote:
> This worries me a little bit.. Are we sure we are
granata.enrico added inline comments.
Comment at: scripts/Python/prepare_binding_Python.py:229
stderr=subprocess.PIPE,
-shell=True)
+)
# Wait for SWIG process to terminate
This worries me a little bit.. Are we sure we are not in any way
granata.enrico added a reviewer: beanz.
granata.enrico added a comment.
Chris, can you take a look at this? I am far from a CMake expert
https://reviews.llvm.org/D26757
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/
ldrumm created this revision.
ldrumm added reviewers: zturner, granata.enrico.
ldrumm added a subscriber: LLDB.
Herald added a subscriber: mgorny.
A combination of broken escaping in CMake and in the python swig generation
scripts meant that the swig generation step would fail whenever there were
19 matches
Mail list logo
