add USD_INCLUDE_DIR to includes #5
Conversation
|
We have always been able to build master and didn't have to call include_directories(${USD_INCLUDE_DIR}) explicitly to the include path. |
|
Odd - I've tried to build it twice now - once with Linux work / luma desktop, once with my own Mac laptop... both failed to find the pixar headers, even though they are passed the USD root (and the find_package works). I'll guess we'll have to see what other people find? |
|
I started looking into building this for Linux on the Pixar side and found that I also had to apply this patch from @elrond79 to get it to find the USD headers. I haven't looked in detail yet to try to figure out what might be different about how we're building it. |
* Make sure the build and install folders have the variant (#2) * Enable generating compile_commands.json by default * Make sure the build and install folders have the variant * Fix typo in ReadMe * temporarily revert line endings to windows to do merge * standardize on linux-style line endings (newline only)
|
Fair enough. It would be nice to understand why things build for some of us and not the others. |
|
Did you mean to close this without merging? |
|
I didn't mean to close it. Not sure how this was closed? I am going to look at a few things tomorrow to see if I can find a logical answer for this. |
|
Can you please send me your build_log.txt when the build fails? |
|
Here's the build_log.txt I get if I don't apply the change in this PR: |
|
Thanks. I am interested to know the list of subdirectories under "/pixar/ws/trees/mattjohnson/usd_open_source/inst" which you have set for PXR_USD_LOCATION variable. Could you add below messages in FindUSD.cmake, right before find_package_handle_standard_args call ( line 70 ) and send me your build.log again? message(STATUS "USD include dir: ${USD_INCLUDE_DIR}") Here is my build log with ${USD_INCLUDE_DIR} ${USD_LIBRARY_DIR} ${USD_VERSION} variables printed out. |
|
Sure thing. Here is the output from those messages: -- USD include dir: /pixar/ws/trees/mattjohnson/usd_open_source/inst/include And that is indeed where core USD is installed. There are files at paths like these: |
|
Good! So the USD include and lib are found and the subdirectories that you are showing looks good to me. The only thing that is a bit troubling is the USD version that you are using. Currently master branch should be build against USD v19.05 and that's what we all have been using. We will be updating the docs to avoid any confusion. Sorry about that! Now, still with v0.19.10, you shouldn't get errors about missing headers. Anyways, it is worth the try if you switch to v0.19.5 One more question, what version of CMake are you using? |
|
Hmm. Well to answer your last question first: We have it installed as cmake3 since we also have a 2.8 version of cmake installed in our environment as As to the USD version, we (Pixar) don't keep older releases of USD around and are effectively always working off of the dev branch. Internally, USD is distributed along with the rest of our software through a different release process, so there is no real mapping between what production is using and a public release version. For developers, we're continually working against and building the latest of both core USD and the Maya/USD integration, so that's the workflow we're aiming to preserve. Changes we check in internally were propagated to the dev branch of the public GitHub repo so that it was usually never more than a few days behind where we were internally. Ideally this means that going forward with this maya-usd repo, we could help anticipate changes in core USD and keep the dev branch of maya-usd building against the latest core USD code. So to the issue here, I dropped the maya-usd repo back to the master branch to illustrate the issue Paul identified here, but once past that, I wouldn't actually expect the master branch to build against 19.10. I have instead been building the dev branch Paul prepared against the very latest USD. I see the same issue in the maya-usd dev branch with core USD includes going missing if we don't add the changes from this PR. I'm not sure any of that helps the issue here, but hopefully that makes sense? :) |
|
I've been tinkering with this a bit more and still don't have a good explanation, but I can confirm that the USD header path is not getting passed into the call to the compiler without this PR. I set the environment variable VERBOSE=1 when running the build script (and also removed the multiproc-ness from the cmake command for more easily diff-able build logs), and without the changes in this PR, I see this for the first file it tries to compile that tries to include a pxr header: When I add the changes from this PR, I see this instead: And for more easily spotting the difference, here are the build commands together: Notice that the only difference is that the second one includes |
|
I'm not sure what is going on, but we should not need to explicitly add this include path. If you look at the cmake/pxrTargets.cmake from (from the USD build) you'll see taht each target has "${PXR_USD_LOCATION}/include" added to it's include directories. So when you link when a Pixar lib, you automatically inherit those include paths. There is another PR against our refactoring_sandbox branch (#8) that fixes something related to the USD paths. Perhaps that will fix this issue too, if we made similar changes to master. |
* Make sure the build and install folders have the variant (#2) * Enable generating compile_commands.json by default * Make sure the build and install folders have the variant * Fix typo in ReadMe * temporarily revert line endings to windows to do merge * standardize on linux-style line endings (newline only) (cherry picked from commit 8073eaf)
|
Ah - thanks Sean, that's a big piece of the puzzle here! Now we know that
the include was added due to the pxrTargets.cmake.
Unfortunately, my pxrTargets.cmake doesn't do this - I'm including it (and
the related installed usd cmake files) in case you see something I don't.
When you said that it should add "${PXR_USD_LOCATION}/include" - did you
mean that it should literally add that string, with that variable? Or was
that a shorthand way of saying that your pxrTargets.cmake adds
"/your/fully/expanded/path/to/usd/include"? Because, as far as I can tell,
PXR_USD_LOCATION doesn't show up anywhere in the USD code base. (Our
pxrConfig.cmake does set PXR_INCLUDE_DIRS - but it's never used after that.)
Matt - could you let us know let us know whether your pxrTargets.cake adds
the USD include dir? If pixar's own USD builds also don't have these
lines, then we should probably not assume that they will be there - even if
they can get there in some configurations / installs - and add them
ourselves.
…On Mon, Aug 12, 2019 at 8:56 AM Sean Donnelly ***@***.***> wrote:
I'm not sure what is going on, but we should not need to explicitly add
this include path. If you look at the cmake/pxrTargets.cmake from (from the
USD build) you'll see taht each target has "${PXR_USD_LOCATION}/include"
added to it's include directories. So when you link when a Pixar lib, you
automatically inherit those include paths.
There is another PR against our refactoring_sandbox branch (#8
<#8>) that fixes something
related to the USD paths. Perhaps that will fix this issue too, if we made
similar changes to master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5?email_source=notifications&email_token=AAARQ6AAGRZ5ZKNVBQEDZA3QEGBZTA5CNFSM4IJBQWGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4C7RMY#issuecomment-520485043>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAARQ6GEMSQ7QGZBNDSJCATQEGBZTANCNFSM4IJBQWGA>
.
|
|
Ah sorry yes when I said "${PXR_USD_LOCATION}/include", what I really meant was "/your/fully/expanded/path/to/usd/include". For example in the target "tf" you should see an INTERFACE_INCLUDE_DIRECTORIES entry like this (after the boost include). FYI, I copy/pasted that from our pxrTargets.cmake where we have actually replaced the full paths with variables so that the USD build is portable. |
|
Oops, just realized that I attached the files to an email reply - I guess github can't cope with that. Anyway - yeah, no such entity is in my pxrTargets.cmake (I added a .txt so github would accept the upload). No obviously "wrong" path there which might be the result of a missing var or something either. pxrConfig.cmake.txt |
|
Ah, thanks for the clues @seando-adsk! My pxrTargets.cmake is also missing any INTERFACE_INCLUDE_DIRECTORIES that point to where I'm building USD. That seems like a potential problem with the core USD build, so I'm following up with some folks internally here to see whether we can address that on the USD side. |
|
I noticed from your files that you are on Linux (above I was referring to the files from Windows - thus the mention of boost). Here is the "tf" section from the "pxrTargets.cmake" file on our Linux machine: set_target_properties(tf PROPERTIES It's strange yours has two pythons and then a TBB, whereas ours has the path (twice) to the USD include folder. Maybe it has to do with the options used when compiling USD? |
|
Ah! You used pixar's.py build script to install / build usd, I'm guessing?
Whereas we (and likely pixar) built frm cmake directly.
I think what's going on is that the build script installs usd and all the
dependencies it builds in the same place. So the "usd" include dir you're
seeing is actually the boost and tbb (or some other dep) include dir - and
usd just happens to be in the same place.
I'll take a look at the macos build I made with the build script, and see
if it looks the same as yours, once I get in.
On Tue, Aug 13, 2019 at 7:30 AM Sean Donnelly <notifications@github.com>
wrote: I noticed from your files that you are on Linux (above I was
referring to the files from Windows - thus the mention of boost). Here is
the "tf" section from the "pxrTargets.cmake" file on our Linux machine:
… set_target_properties(tf PROPERTIES
INTERFACE_COMPILE_DEFINITIONS "PXR_PYTHON_ENABLED=1"
INTERFACE_INCLUDE_DIRECTORIES
"/usr/include/python2.7;/local/S/jenkins/workspace/ecg-usd-full-linux/build/RelWithDebInfo/include;/local/S/jenkins/workspace/ecg-usd-full-linux/build/RelWithDebInfo/include"
)
It's strange yours has two pythons and then a TBB, whereas ours has the
path (twice) to the USD include folder. Maybe it has to do with the options
used when compiling USD?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5?email_source=notifications&email_token=AAARQ6B5K2S6EJH4SH7IWPLQELAOTA5CNFSM4IJBQWGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4F24XA#issuecomment-520859228>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAARQ6AH6DJDJFST3TS37NLQELAOTANCNFSM4IJBQWGA>
.
|
|
So, my macos build does have the same dir for the last 2 dirs… but it’s NOT
the same as the one USD is in.
However, IIRC, that IS a configurable option - ie, you can choose to
install USD and all it’s dependencies in the same place. Sean, would you
mind reporting on whether this dir:
/local/S/jenkins/workspace/ecg-usd-full-linux/build/RelWithDebInfo/include
…also includes the boost and tbb headers?
If so, that would fit with what we’ve seen so far: all the tf
INTERFACE_INCLUDE_DIRECTORIES I’ve seen so far have 3 directories, and my
hunch is that they’re always:
python/include;boost/include;tbb/include
Sean - in my pxrTargets.cmake, the 2nd entry which looked like a
python/include is actually a boost include - the dir was:
/luma/soft/rez/packages/boost/1.61.0/platform-linux/gcc-6.3.1/python-2.7/include;
…which looks like a python dir, because that’s the dir immediately before
the “/include” - but it’s a rez-built package, which has all it’s
dependencies in the directory structure… so it’s actually a boost install,
that was built against linux, gcc-6.3.1, and python-2.7.
…On Tue, Aug 13, 2019 at 7:48 AM Paul Molodowitch ***@***.***> wrote:
Ah! You used pixar's.py build script to install / build usd, I'm guessing?
Whereas we (and likely pixar) built frm cmake directly.
I think what's going on is that the build script installs usd and all the
dependencies it builds in the same place. So the "usd" include dir you're
seeing is actually the boost and tbb (or some other dep) include dir - and
usd just happens to be in the same place.
I'll take a look at the macos build I made with the build script, and see
if it looks the same as yours, once I get in.
On Tue, Aug 13, 2019 at 7:30 AM Sean Donnelly ***@***.***>
wrote: I noticed from your files that you are on Linux (above I was
referring to the files from Windows - thus the mention of boost). Here is
the "tf" section from the "pxrTargets.cmake" file on our Linux machine:
> set_target_properties(tf PROPERTIES
> INTERFACE_COMPILE_DEFINITIONS "PXR_PYTHON_ENABLED=1"
> INTERFACE_INCLUDE_DIRECTORIES
> "/usr/include/python2.7;/local/S/jenkins/workspace/ecg-usd-full-linux/build/RelWithDebInfo/include;/local/S/jenkins/workspace/ecg-usd-full-linux/build/RelWithDebInfo/include"
> )
>
> It's strange yours has two pythons and then a TBB, whereas ours has the
> path (twice) to the USD include folder. Maybe it has to do with the options
> used when compiling USD?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#5?email_source=notifications&email_token=AAARQ6B5K2S6EJH4SH7IWPLQELAOTA5CNFSM4IJBQWGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4F24XA#issuecomment-520859228>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAARQ6AH6DJDJFST3TS37NLQELAOTANCNFSM4IJBQWGA>
> .
>
|
|
Yes we are using "usd/build_scripts/build_usd.py" to build USD. And yes, our include folder includes: boost, GL, OpenEXR, opensubdiv, pxr, serial and tbb. We are currently letting USD download and build all of it's dependencies, such as boost and tbb. Sean |
|
I found a solution for this on the core USD side that ensures the correct header include path is written into the pxrTargets.cmake. It should make its way out to the public USD repo soon, but until then, here's the patch: Building core USD with that change, I was then able to build maya-usd against it without the changes in this PR. @elrond79, when you get a chance, can you give this a shot and see if it resolves the problem for you too? |
|
Yes, with this change to USD core, I can build without this PR! Having said that, we still need this PR for master, which is targeting usd-19.05, which obviously doesn't have this USD fix. Once the fix makes it onto a public USD push, then we can drop it from the dev branch... but in the meantime, as Sean's own header include path demonstrates, having some duplicate entries in the include path isn't a big deal. (And as issue #19 demonstrates, people are already running into this problem.) |
|
Yup, great point. Just wanted to confirm that the core USD build change sorted out the dev build for you too. Thanks! |
|
I'm going to drop this, since we're "soon" going to be requiring usd-0.19.11 (The exception being AL's usd-0.19.01 PR, but that's basically just to accomodate them, and I don't think they encountered this particular problem...) |
So, this may not be necessary / may be due to a problem on my part. But when I tried to build, it immediately failed, because none of the USD headers could be find. I couldn't find anywhere in the CMakeLists files that seemed to explicitly add the USD_INCLUDE_DIR to the include path... and when I did, it worked.
However, I assume there must be some other way this is being done for you folks, or else you'd obviously never have been able to build... so maybe I'm missing something?