Closed phniix closed 3 years ago
There is a fair bit going on in the documentation generation, so here are my ideas on a piecewise transition from Python to CMake.
I think the juice is worth the squeeze. I'd love to hear your thoughts. It would be great to find out if some of these ideas resonate with you.
CMake provides a Doxygen find package module, which dates back to the minimum CMake version requirements for this project. 3.1.1 - https://github.com/Kitware/CMake/blob/v3.1.1/Modules/FindDoxygen.cmake 2.8.8 - https://cmake.org/cmake/help/v2.8.8/cmake.html#module:FindDoxygen
The Doxygen find_package() module cleverly enforces the default need for dot
to be available on the system. As a result, we can utilise this CMake package to trim down the code in CMakeLists.txt
, as well as the code in generateDocs.py
.
Doxyfile.in
would need a small tweak to account for the variable name change for tag DOT_PATH
.
We can add additional commands to the custom target 'documentation' in CMakeLists.txt
. This will then remove the need to have generateDocs.py
execute doxygen as a subprocess.
An added benefit of this approach is that warnings and errors encountered by doxygen will be displayed as part of the build process. Currently, running the doxygen Doxyfile
manually will highlight warnings and errors (some of which are false positives!), however these are lost in the generateDocs.py
execution of the implementation.
There are a lot of file copy operations which occur in order to conform the source file layout to the expectation set for the INPUT tag in Doxyfile.in.
As an aside: There may be a discussion on how this may be better managed so as to reduce inode count.. such a conversation would of course be welcome, and devops teams are encouraged to share their experiences and thoughts!
The module flattening and file copy operations in generateDocs.py
are able to be transposed from Python to CMake. File operations in Python that can also be moved into CMake as additional commands to the custom target 'documentation'. We can also approach removing some file copy operations if we take a slightly different approach on how we execute the doxygen build command.
A first pass on the above ideas can be seen here: https://github.com/phniix/USD/commit/75be658bb0746dc949d0c7d8c446d4d625e77bd3
In this first pass, the module flattening code remains in Python.
I stop short of transposing it to CMake, as there may be alternatives to the GLOB style approach currently being employed. Alternatives could be a myriad of concepts. One alternative could be to specify explicitly the modules that are to be documented. This could, for example be through the declarations pxr_library()
, pxr_shared_library()
.
These specific declarations are the mechanism for the container which would be used to populate the 'INPUT' tag in Doxyfile.in
, thus automating this process so the values always remain current. For example, with the changes in this commit, I have the following report from a build:
λ cmake --build . --target documentation --config Release
... snip ...
CUSTOMBUILD : warning : tag INPUT: input source `src/modules/gal' does not exist [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : tag INPUT: input source `src/modules/glfq' does not exist [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : tag INPUT: input source `src/modules/gusd/overview.dox' does not exist [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : source src/modules/gal is not a readable file or directory... skipping. [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : source src/modules/glfq is not a readable file or directory... skipping. [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : source src/modules/gusd/overview.dox is not a readable file or directory... skipping. [D:\gitrepos\USD\build\documentation.vcxproj]
... snip ...
And while yes, having these warnings now exposed may trigger a quick change to the relevant files, it might be nice to not have to worry about it ever again! Furthermore, such a container will know the exact files needed to be copied over for the src/modules
directory, thus reducing the copy overhead and improving the documentation build performance.
Having just said that, the specific page ordering required by the docs does indeed make it a little bit more challenging to articulate the requirement through declarations.
All that aside, I think it's a fun first pass, and I hope you do too!
P.S. The above changes will result in 2 errors reported by the documentation build, as seen below.
"D:\gitrepos\USD\build\documentation.vcxproj" (default target) (1) ->
(CustomBuild target) ->
CUSTOMBUILD : error : Problems running latex. Check your installation or look for typos in _formulas.tex and check _formulas.log! [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : error : Problems running dvips. Check your installation! [D:\gitrepos\USD\build\documentation.vcxproj]
I have addressed these two errors through issue #719. Apply the changes as described in that issue, and the errors should resolve.
Why can't the INPUT tag point directly at files in the git repository? This would have the advantage that Doxygen errors would actually contain the name of the file that needs to be fixed, as well as avoiding a lot of copying. Failing that I would recommend symbolic links (they work on Windows nowadays, right?)
On Fri, Dec 7, 2018 at 10:40 PM pheonix notifications@github.com wrote:
A first pass on the above ideas can be seen here: phniix@75be658 https://github.com/phniix/USD/commit/75be658bb0746dc949d0c7d8c446d4d625e77bd3
In this first pass, the module flattening code remains in Python.
I stop short of transposing it to CMake, as there may be alternatives to the GLOB style approach currently being employed. Alternatives could be a myriad of concepts. One alternative could be to specify explicitly the modules that are to be documented. This could, for example be through the declarations pxr_library(), pxr_shared_library().
These specific declarations are the mechanism for the container which would be used to populate the 'INPUT' tag in Doxyfile.in, thus automating this process so the values always remain current. For example, with the changes in this commit, I have the following report from a build:
λ cmake --build . --target documentation --config Release
... snip ...
CUSTOMBUILD : warning : tag INPUT: input source `src/modules/gal' does not exist [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : tag INPUT: input source `src/modules/glfq' does not exist [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : tag INPUT: input source `src/modules/gusd/overview.dox' does not exist [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : source src/modules/gal is not a readable file or directory... skipping. [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : source src/modules/glfq is not a readable file or directory... skipping. [D:\gitrepos\USD\build\documentation.vcxproj]
CUSTOMBUILD : warning : source src/modules/gusd/overview.dox is not a readable file or directory... skipping. [D:\gitrepos\USD\build\documentation.vcxproj]
... snip ...
And while yes, having these warnings now exposed may trigger a quick change to the relevant files, it might be nice to not have to worry about it ever again! Furthermore, such a container will know the exact files needed to be copied over for the src/modules directory, thus reducing the copy overhead and improving the documentation build performance.
Having just said that, the specific page ordering required by the docs does indeed make it a little bit more challenging to articulate the requirement through declarations.
All that aside, I think its a fun first pass, and I hope you do too!
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/PixarAnimationStudios/USD/issues/718#issuecomment-445436502, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLBEdKdX_dpKcNF3kotDTvRjFAhmzJLks5u216_gaJpZM4ZJZKf .
That's a really good question @spitzak.
It was one of my original notions for files being used for documentation generation to be added through CMake's configure_file()
, and the spot for that, as you have rightfully pointed out, being the INPUT tag.
Delving into what exists currently in the CMakeLists.txt files, not every header file is explicitly named. A cursory assessment such as the following shows what I mean.
(I'm doing this while sitting on SHA1 79f095a)
pxr directory Header file declarations.
$ find pxr/ -type f -name "CMakeLists.txt" | xargs -I {} ack -H "\.h" {}
pxr/base/lib/arch/CMakeLists.txt
1:# It's important to not include the wrong math.h here.
42: align.h
43: api.h
44: buildMode.h
45: defines.h
46: export.h
47: functionLite.h
48: hints.h
49: inttypes.h
50: math.h
51: pragmas.h
54: testArchAbi.h
55: testArchUtil.h
pxr/base/lib/gf/CMakeLists.txt
1:# Don't get the wrong half.h!
68: api.h
69: declare.h
70: ilmbase_halfLimits.h
71: limits.h
72: traits.h
78: ilmbase_eLut.h
79: ilmbase_toFloat.h
pxr/base/lib/js/CMakeLists.txt
17: api.h
18: converter.h
19: types.h
pxr/base/lib/plug/CMakeLists.txt
31: api.h
32: thisPlugin.h
pxr/base/lib/tf/CMakeLists.txt
1:# Make sure not to get the wrong semaphore.h
120: api.h
121: callContext.h
122: cxxCast.h
123: declarePtrs.h
124: diagnosticLite.h
125: hashmap.h
126: hashset.h
127: instantiateSingleton.h
128: instantiateStacked.h
129: instantiateType.h
130: preprocessorUtils.h
131: preprocessorUtilsLite.h
132: safeTypeCompare.h
133: staticData.h
134: staticTokens.h
135: typeInfoMap.h
136: type_Impl.h
139: pyStaticTokens.h
150: scopeDescriptionPrivate.h
151: pxrDoubleConversion/ieee.h
152: pxrDoubleConversion/utils.h
153: pxrDoubleConversion/double-conversion.h
154: pxrDoubleConversion/bignum-dtoa.h
155: pxrDoubleConversion/bignum.h
156: pxrDoubleConversion/cached-powers.h
157: pxrDoubleConversion/diy-fp.h
158: pxrDoubleConversion/fast-dtoa.h
159: pxrDoubleConversion/fixed-dtoa.h
160: pxrDoubleConversion/strtod.h
161: pxrLZ4/lz4.h
pxr/base/lib/trace/CMakeLists.txt
48: api.h
49: concurrentList.h
50: stringHash.h
51: trace.h
pxr/base/lib/vt/CMakeLists.txt
34: api.h
35: traits.h
36: operators.h
39: pyOperators.h
42: typeHeaders.h
pxr/base/lib/work/CMakeLists.txt
26: api.h
pxr/imaging/lib/cameraUtil/CMakeLists.txt
17: api.h
pxr/imaging/lib/garch/CMakeLists.txt
41: api.h
42: gl.h
43: glPlatformContext.h
44: ${GARCH_GLPLATFORMCONTEXT}.h
47: ${GARCH_GLPLATFORMDEBUGWINDOW}.h
pxr/imaging/lib/glf/CMakeLists.txt
98: api.h
101: rankedTypeMap.h
102: stb/stb_image.h
103: stb/stb_image_resize.h
104: stb/stb_image_write.h
pxr/imaging/lib/hd/CMakeLists.txt
99: api.h
100: version.h
pxr/imaging/lib/hdSt/CMakeLists.txt
87: api.h
pxr/imaging/lib/hdx/CMakeLists.txt
51: shadowMatrixComputation.h
52: version.h
53: api.h
60: unitTestGLDrawing.h
61: unitTestUtils.h
pxr/imaging/lib/hf/CMakeLists.txt
13: api.h
14: diagnostic.h
15: pluginDesc.h
16: perfLog.h
pxr/imaging/lib/pxOsd/CMakeLists.txt
15: api.h
pxr/imaging/plugin/hdEmbree/CMakeLists.txt
44: context.h
45: renderParam.h
pxr/usd/lib/ar/CMakeLists.txt
16: api.h
17: asset.h
18: assetInfo.h
19: defaultResolver.h
20: defaultResolverContext.h
21: filesystemAsset.h
22: definePackageResolver.h
23: defineResolver.h
24: defineResolverContext.h
25: packageResolver.h
26: packageUtils.h
27: resolver.h
28: resolverContext.h
29: resolverContextBinder.h
30: resolverScopedCache.h
31: threadLocalScopedCache.h
34: pyResolverContext.h
37: debugCodes.h
pxr/usd/lib/kind/CMakeLists.txt
13: api.h
pxr/usd/lib/ndr/CMakeLists.txt
29: api.h
30: nodeDiscoveryResult.h
pxr/usd/lib/pcp/CMakeLists.txt
20: api.h
pxr/usd/lib/sdf/CMakeLists.txt
91: api.h
92: accessorHelpers.h
93: declareSpec.h
94: schemaTypeRegistration.h
117: valueTypePrivate.h
pxr/usd/lib/sdr/CMakeLists.txt
17: api.h
18: declare.h
pxr/usd/lib/usd/CMakeLists.txt
83: api.h
86: crateDataTypes.h
87: crateValueInliners.h
88: wrapUtils.h
138: codegenTemplates/api.h
141: codegenTemplates/schemaClass.h
143: codegenTemplates/tokens.h
pxr/usd/lib/usdGeom/CMakeLists.txt
58: api.h
pxr/usd/lib/usdHydra/CMakeLists.txt
16: api.h
pxr/usd/lib/usdLux/CMakeLists.txt
30: api.h
pxr/usd/lib/usdRi/CMakeLists.txt
43: api.h
pxr/usd/lib/usdShade/CMakeLists.txt
29: api.h
pxr/usd/lib/usdSkel/CMakeLists.txt
41: api.h
42: binding.h
pxr/usd/lib/usdUI/CMakeLists.txt
18: api.h
pxr/usd/lib/usdUtils/CMakeLists.txt
32: api.h
pxr/usd/lib/usdVol/CMakeLists.txt
20: api.h
pxr/usd/plugin/sdrOsl/CMakeLists.txt
28: api.h
pxr/usd/plugin/usdAbc/CMakeLists.txt
48: api.h
pxr/usd/plugin/usdMtlx/CMakeLists.txt
37: api.h
pxr/usdImaging/lib/usdImaging/CMakeLists.txt
67: api.h
68: unitTestHelper.h
69: version.h
pxr/usdImaging/lib/usdImagingGL/CMakeLists.txt
54: api.h
55: rendererSettings.h
56: version.h
63: unitTestGLDrawing.h
pxr/usdImaging/lib/usdShaders/CMakeLists.txt
15: api.h
pxr/usdImaging/lib/usdSkelImaging/CMakeLists.txt
16: api.h
pxr/usdImaging/lib/usdviewq/CMakeLists.txt
25: api.h
Header file count.
$ find pxr -type f -name "*.h" | wc -l
1093
third_party directory Header file declarations.
$ find third_party/ -type f -name "CMakeLists.txt" | xargs -I {} ack -H "\.h" {}
third_party/houdini/lib/gusd/CMakeLists.txt
80: context.h
81: api.h
82: defaultArray.h
83: gusd.h
84: GU_USD.h
85: stageOpts.h
86: USD_PropertyMap.h
87: USD_TraverseSimple.h
88: UT_Assert.h
89: UT_CappedCache.h
90: UT_StaticInit.h
91: UT_Version.h
92: UT_Gf.h
93: GT_VtArray.h
96: GT_VtStringArray.h
97: UT_VtArray.h
third_party/houdini/plugin/OP_gusd/CMakeLists.txt
48: SOP_usdexportattributes.hda
49: SOP_usdinstanceprototypes.hda
50: SOP_usdretime.hda
third_party/katana/lib/katanaPluginApi/CMakeLists.txt
9: FnAsset.h
10: FnAttributeModifier.h
11: FnScenegraphGenerator.h
12: FnViewerModifier.h
13: FnViewerModifierInput.h
14: ScenegraphLocationDelegate.h
third_party/katana/lib/usdKatana/CMakeLists.txt
61: api.h
third_party/katana/plugin/pxrUsdInPrman/CMakeLists.txt
15: declarePackageOps.h
third_party/katana/plugin/pxrUsdInShipped/CMakeLists.txt
21: declareCoreOps.h
third_party/maya/lib/pxrUsdMayaGL/CMakeLists.txt
34: api.h
35: renderParams.h
third_party/maya/lib/px_vp20/CMakeLists.txt
27: api.h
third_party/maya/lib/usdMaya/CMakeLists.txt
32: api.h
112: shadingModePxrRis_rfm_map.h
third_party/maya/plugin/pxrUsd/CMakeLists.txt
24: api.h
third_party/maya/plugin/pxrUsdTranslators/CMakeLists.txt
51: api.h
Header file count.
$ find third_party -type f -name "*.h" | wc -l
203
Due to not every header being named, there would need to be some changes in the way that the CMake scripts are collating required files. Taking a look at the implementation of those pxr_libary()
-esque commands in Public.cmake
and Private.cmake
, there is no clear global list of headers being constructed; nor is there the existence of a global cached variable storing them. The depths of those commands takes on a pretty funky approach for getting the libraries built; as only necessary files are copied for the build. The approach for documentation is different.
As a result, it unfortunately strips away an easy-peasy approach that may go something like:
INPUT = @PXR_ALL_PUBLIC_HEADER_FILES@ \
@PXR_ALL_PRIVATE_HEADER_FILES@
With all that said, we do need to take a look at the current Doxyfile configuration.
Currently, USD's doxygen configuration template file, Doxyfile.in, declares a specific ordering of files mixed with directory names. It uses Doxygen's recursive pattern match to drill down and collate relevant files for document generation.
This specific ordering is vital in order to align the documentation with the Project's intended communication style and approach.
This adds a complexity to the named file or directory via CMake's configure_file()
approach. That easy-peasy approach I mentioned earlier would definitely not work with these requirements.
It is plausible for an adjustment to be made to the current Doxyfile.in
specifying subdirectories in such a way that they reference the library and plugin directories for each library's module in the repository. As @spitzak points out, this approach would get Doxygen warnings and errors referencing the actual file that needs to be fixed. It's not an automagical approach though :(
It is a balance of sorts, and I see why the current approach has been taken. I too am left wondering if we can avoid major file copy operations altogether, and reference original file locations ?
On the point of using symbolic links as a fallback option, whilst remembering that the goal is to remove the Python build time dependency for document generation, CMake currently only supports this operation on Unix systems, ie: cmake -E create_symlink ${target} ${link}
. Of course, something can be done for Windows to use mklink
.. on those versions of Windows that support it, as well as those supported by this project. While this leads us to messy territory handling cross-platform code, if we must, then we must. But the question will beckon, must we ?
Having said that, symbolic links will still eat up inodes, and the less inodes consumed by this process, the better.
Filed as internal issue #USD-4978.
Kindly upload Pixar USD maya Xcode project source?
@usmanredsoft This being a CMake project, any platform specific project sources like those that would be for Xcode, would be generated by CMake. Are you able to elaborate a little bit more, maybe I am not understanding your question ?
Just wanted to mention that I had an opportunity to revisit this as part of other work I'm doing and was able to modify the build to no longer rely on the generateDocs.py script. This should get pushed out soon!
This is tremendously great news! We are all looking forward to this!
Thank you :)
(@sunyab)
Description of Issue
The documentation generation process has a dependency on Python. As part of the build process, it would be nice if we could have a cross-platform solution that doesn't depend on Python being available on a host build system.
Recently, a convo in issue #605 discussed the build time Python dependency for copying header files. The general feeling that we flowed towards was removing all Python build time dependencies, unless specifically asking to build with Python support. As a result, the Python dependency on document generation was brought up, something I thought would be worthwhile pursuing here in this separately tracked issue.
We would like to discuss the transition of the current docgen code from Python to CMake.
Current Python docgen code: https://github.com/PixarAnimationStudios/USD/blob/master/cmake/macros/generateDocs.py Current CMake docgen code utilising the above Python docgen code: https://github.com/PixarAnimationStudios/USD/blob/6c50b29bb76f0a7c176bae05668ab195520a019b/CMakeLists.txt#L70
From our convo on this topic in issue #605 , a question we did raise was: is the juice worth the squeeze ?
Steps to Reproduce
Make sure that the minimum requirements for document generation have been met by the build system you are on. See Optional Components - Documentation. These tools need to be on your PATH (*nix,darwin|win), or in your REGISTRY (win).
Generate build files from a build directory.
Build documentation
System Information (OS, Hardware)
Windows 10 Home - 10.0.17134 Build 17134 Visual Studio 2017 Community v15.8.8 - CL.exe (x64) 19.15.26732.1
USD dev branch. Commit SHA1: 79f095a
Package Versions
Doxygen 1.8.4, 1.8.6, 1.8.14 * (I did try this using v1.8.4 because that's the version reported in Doxyfile.in) Graphviz 2.38.0 Python 3.7.0 CMake 3.11.4 Boost 1.68 TBB 2018 U5
Build Flags
Command Log Dump
Build file generation.
Document Generation