Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(165)

Issue 7376056: i#1091: Work around PARENT_SCOPE issues with macros instead of functions (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 1 month ago by Reid Kleckner (google)
Modified:
13 years, 1 month ago
Reviewers:
bruening
CC:
dynamorio-devs_googlegroups.com
Base URL:
https://dynamorio.googlecode.com/svn/trunk
Visibility:
Public.

Description

i#1091: Work around PARENT_SCOPE issues with macros instead of functions Fixes the sample client symbol visibility on Linux, but doesn't solve the larger PARENT_SCOPE issues in configure_DynamoRIO_global().

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -35 lines) Patch
M api/samples/CMakeLists.txt View 5 chunks +39 lines, -35 lines 2 comments Download

Messages

Total messages: 3
Reid Kleckner (google)
13 years, 1 month ago (2013-02-26 14:50:12 UTC) #1
bruening
https://codereview.appspot.com/7376056/diff/1/api/samples/CMakeLists.txt File api/samples/CMakeLists.txt (right): https://codereview.appspot.com/7376056/diff/1/api/samples/CMakeLists.txt#newcode130 api/samples/CMakeLists.txt:130: if (NOT DEFINED INSTALL_PREFIX) this can't go above the ...
13 years, 1 month ago (2013-02-26 19:00:10 UTC) #2
Reid Kleckner (google)
13 years, 1 month ago (2013-02-26 19:10:57 UTC) #3
On Tue, Feb 26, 2013 at 2:00 PM, <bruening@google.com> wrote:

>
> https://codereview.appspot.**com/7376056/diff/1/api/**
>
samples/CMakeLists.txt<https://codereview.appspot.com/7376056/diff/1/api/samples/CMakeLists.txt>
> File api/samples/CMakeLists.txt (right):
>
> https://codereview.appspot.**com/7376056/diff/1/api/**
>
samples/CMakeLists.txt#**newcode130<https://codereview.appspot.com/7376056/diff/1/api/samples/CMakeLists.txt#newcode130>
> api/samples/CMakeLists.txt:**130: if (NOT DEFINED INSTALL_PREFIX)
> this can't go above the strip point
>
> https://codereview.appspot.**com/7376056/diff/1/api/**
>
samples/CMakeLists.txt#**newcode180<https://codereview.appspot.com/7376056/diff/1/api/samples/CMakeLists.txt#newcode180>
> api/samples/CMakeLists.txt:**180: # FIXME i#1091: use a macro instead of a
>
> function to work around issues with
> let's solve the real problem.  I can't tell from i#1091 what the problem
> is.  why didn't my i#1052 fix solve it?
>

The current status is that deep in configure_DynamoRIO_global(), we set
*CMAKE_*_FLAGS*, and then propagate it backwards with set(PARENT_SCOPE).

The problem is that when we call configure_DynamoRIO_client() inside
function scope, those variables go away on the next function call (no more
C_FLAGS), but the global property stays the same, so we skip the global
config.

We could spot fix it by copy-pasting the for loop that pops those variables
up to the next scope into api/samples/CMakeLists.txt, but that seems
unappealing.

My plan was to commit the workaround, and then reassign the bug to you,
since you wrote this stuff.  =D
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b