Update documentation setup configuration #99

Krishna-13-cyber · 2023-06-28T18:33:47Z

No description provided.

vgvassilev · 2023-06-28T22:41:15Z

Why do we need llvm 10? CppInterOp will require some work to enable llvm 10 if possible at all.

Krishna-13-cyber · 2023-06-29T09:02:10Z

Why do we need llvm 10? CppInterOp will require some work to enable llvm 10 if possible at all.

The readthedocs supports llvm-version only till llvm-10 and also prior cmake versions.So I had to use these versions for readthedocs compatibility.

vgvassilev · 2023-06-29T09:05:05Z

Do we really need to build the codebase with llvm-10? I think if we specified the doxygen build flag we should probably ignore the llvm version checks as we could be only building the doxygen config.

davidlange6 · 2023-06-29T09:11:24Z

Where can we see this llvm-10 support? I don't immediately find it

…

On Jun 29, 2023, at 11:02 AM, Krishna Narayanan ***@***.***> wrote: Why do we need llvm 10? CppInterOp will require some work to enable llvm 10 if possible at all. The readthedocs supports llvm-version only till llvm-10 and also prior cmake versions.So I had to use these versions for readthedocs compatibilty. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

Krishna-13-cyber · 2023-06-29T09:29:21Z

Do we really need to build the codebase with llvm-10? I think if we specified the doxygen build flag we should probably ignore the llvm version checks as we could be only building the doxygen config.

We just need llvm to pass the cmake checks, it is nothing to do with docs generation. I will try to get some alter method for this

Krishna-13-cyber · 2023-06-29T09:34:41Z

Where can we see this llvm-10 support? I don't immediately find it

The llvm-10 support was working for clad but I had upgraded it for interop to llvm-14 but it didnt seem to work well.
Yes, I found it too while building the docs in readthedocs build setup, I will just share a screen capture of build failure (llvm version incompatible).

davidlange6 · 2023-06-29T09:35:03Z

whose cmake checks?

davidlange6 · 2023-06-29T09:35:46Z

ok, I see your graphic now..

davidlange6 · 2023-06-29T09:40:33Z

did you consider using conda to do the installs instead?

parth-07 · 2023-07-02T07:50:43Z

@vgvassilev @davidlange6 If I understand correctly, the pull request is not actually building CppInterOp in the readthedocs configuration. llvm-10 is only required to run CMake on the CppInterOp. CMake will generate documentation targets. These documentation targets are used to generate sphinx and doxygen docs.

I think if we specified the doxygen build flag we should probably ignore the llvm version checks as we could be only building the doxygen config.

Yes, this can be done. For this, we will have to add an option such as,
CPPINTEROP_BUILD_ONLY_DOCS. This option can modify CMake to explicitly ignore all configurations
related to building the project. But I don't think this option is required for this change, as we can use llvm-10
to bypass the CMake checks.

did you consider using conda to do the installs instead?

CppInterOp is not being built / installed. Only documentation is being built.

parth-07 · 2023-07-02T07:52:30Z

CMakeLists.txt

-  if (NOT LLVM_FOUND AND DEFINED LLVM_DIR)
-    find_package(LLVM REQUIRED CONFIG ${llvm_search_hints} NO_DEFAULT_PATH)
-  endif()
+  # if (NOT LLVM_FOUND AND DEFINED LLVM_DIR)


I think this change can be reverted now.

parth-07 · 2023-07-02T07:52:43Z

CMakeLists.txt

-  if (NOT Clang_FOUND AND DEFINED Clang_DIR)
-    find_package(Clang REQUIRED CONFIG ${clang_extra_hints} NO_DEFAULT_PATH)
-  endif()
+  # if (NOT Clang_FOUND AND DEFINED Clang_DIR)


This can be reverted as well.

vgvassilev · 2023-07-02T08:27:58Z

@vgvassilev @davidlange6 If I understand correctly, the pull request is not actually building CppInterOp in the readthedocs configuration. llvm-10 is only required to run CMake on the CppInterOp. CMake will generate documentation targets. These documentation targets are used to generate sphinx and doxygen docs.

I think if we specified the doxygen build flag we should probably ignore the llvm version checks as we could be only building the doxygen config.

Yes, this can be done. For this, we will have to add an option such as, CPPINTEROP_BUILD_ONLY_DOCS. This option can modify CMake to explicitly ignore all configurations related to building the project. But I don't think this option is required for this change, as we can use llvm-10 to bypass the CMake checks.

did you consider using conda to do the installs instead?

CppInterOp is not being built / installed. Only documentation is being built.

Yes, but that change would make it look like we support llvm-10 which we do not. So users with llvm-10 might trigger a build which would then fail. That’s what I am trying to avoid here. The best would be to install the right llvm in the container. The second best would be to use a special cmake flag.

parth-07 · 2023-07-02T10:35:48Z

Yes, but that change would make it look like we support llvm-10 which we do not.

Oh, yes you are right.

The best would be to install the right llvm in the container. The second best would be to use a special cmake flag.

Yes, this seems right. @Krishna-13-cyber can you please add the special CMake flag for this if we cannot install
right llvm versions?

Krishna-13-cyber · 2023-07-02T12:09:03Z

Yes, this seems right. @Krishna-13-cyber can you please add the special CMake flag for this if we cannot install right llvm versions?

Yes, I will get it done!

davidlange6 · 2023-07-03T09:58:29Z

"llvm-10 is only required to run CMake on the CppInterOp." - this doesn't make much sense to me. Whose limitation is this?

vgvassilev · 2023-07-03T10:00:57Z

"llvm-10 is only required to run CMake on the CppInterOp." - this doesn't make much sense to me. Whose limitation is this?

Apparently that's what's in the default container of the readthedocs infrastructure. So that limitation comes from there and I am not even convinced if that's a limitation per se. What we should not do is change our cmake files to seem that we support llvm-10 as that'd be incorrect.

davidlange6 · 2023-07-03T10:09:00Z

ok, I guess this is because build.image is basically obsolete. I'd suggest we follow build.os instead..
https://docs.readthedocs.io/en/stable/tutorial/#adding-a-configuration-file

Krishna-13-cyber · 2023-07-06T05:38:48Z

ok, I guess this is because build.image is basically obsolete. I'd suggest we follow build.os instead.. https://docs.readthedocs.io/en/stable/tutorial/#adding-a-configuration-file

I have used this approach and works perfect. Thanks!

vgvassilev · 2023-07-06T05:42:12Z

Can you configure your editor to remove trailing white space?

Krishna-13-cyber · 2023-07-06T09:10:08Z

Can you configure your editor to remove trailing white space?

Yes, I will do this but I don't see any trailing spaces here in this PR!

vgvassilev · 2023-07-12T08:33:28Z

docs/conf.py


-INTEROP_ROOT = '..'
+html_theme_options = {
+    "github_user": "vgvassilev",


We should probably use compiler-research.

vgvassilev

LGTM!

parth-07 suggested changes Jul 2, 2023

View reviewed changes

vgvassilev reviewed Jul 12, 2023

View reviewed changes

Update changes for documentation configuration

94d1c7f

Krishna-13-cyber force-pushed the update-docs-setup branch from 09e61b7 to 94d1c7f Compare July 12, 2023 15:18

vgvassilev approved these changes Jul 12, 2023

View reviewed changes

vgvassilev added this pull request to the merge queue Jul 12, 2023

Merged via the queue into compiler-research:main with commit e087953 Jul 12, 2023
3 of 4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update documentation setup configuration #99

Update documentation setup configuration #99

Krishna-13-cyber commented Jun 28, 2023

vgvassilev commented Jun 28, 2023

Krishna-13-cyber commented Jun 29, 2023 •

edited

vgvassilev commented Jun 29, 2023

davidlange6 commented Jun 29, 2023 via email

Krishna-13-cyber commented Jun 29, 2023

Krishna-13-cyber commented Jun 29, 2023

davidlange6 commented Jun 29, 2023

davidlange6 commented Jun 29, 2023

davidlange6 commented Jun 29, 2023

parth-07 commented Jul 2, 2023 •

edited

parth-07 Jul 2, 2023

parth-07 Jul 2, 2023

vgvassilev commented Jul 2, 2023

parth-07 commented Jul 2, 2023

Krishna-13-cyber commented Jul 2, 2023

davidlange6 commented Jul 3, 2023

vgvassilev commented Jul 3, 2023

davidlange6 commented Jul 3, 2023

Krishna-13-cyber commented Jul 6, 2023 •

edited

vgvassilev commented Jul 6, 2023

Krishna-13-cyber commented Jul 6, 2023

vgvassilev Jul 12, 2023

vgvassilev left a comment

Update documentation setup configuration #99

Update documentation setup configuration #99

Conversation

Krishna-13-cyber commented Jun 28, 2023

vgvassilev commented Jun 28, 2023

Krishna-13-cyber commented Jun 29, 2023 • edited

vgvassilev commented Jun 29, 2023

davidlange6 commented Jun 29, 2023 via email

Krishna-13-cyber commented Jun 29, 2023

Krishna-13-cyber commented Jun 29, 2023

davidlange6 commented Jun 29, 2023

davidlange6 commented Jun 29, 2023

davidlange6 commented Jun 29, 2023

parth-07 commented Jul 2, 2023 • edited

parth-07 Jul 2, 2023

Choose a reason for hiding this comment

parth-07 Jul 2, 2023

Choose a reason for hiding this comment

vgvassilev commented Jul 2, 2023

parth-07 commented Jul 2, 2023

Krishna-13-cyber commented Jul 2, 2023

davidlange6 commented Jul 3, 2023

vgvassilev commented Jul 3, 2023

davidlange6 commented Jul 3, 2023

Krishna-13-cyber commented Jul 6, 2023 • edited

vgvassilev commented Jul 6, 2023

Krishna-13-cyber commented Jul 6, 2023

vgvassilev Jul 12, 2023

Choose a reason for hiding this comment

vgvassilev left a comment

Choose a reason for hiding this comment

Krishna-13-cyber commented Jun 29, 2023 •

edited

parth-07 commented Jul 2, 2023 •

edited

Krishna-13-cyber commented Jul 6, 2023 •

edited