[clang-doc] fix bug introduced by asset test #97540

PeterChou1 · 2024-07-03T08:45:04Z

Fixes #97507
this patch fixes the bug by copying assets to the correct repository for windows builds. It introduces a new global variable
LLVM_SHARE_OUTPUT_INTDIR which points a platform agnostic shared directory

llvmbot · 2024-07-03T08:45:35Z

@llvm/pr-subscribers-clang-tools-extra

Author: None (PeterChou1)

Changes

Fixes #97507

this patch fixes the bug by copying assets to the correct repository for windows builds

Full diff: https://github.com/llvm/llvm-project/pull/97540.diff

1 Files Affected:

(modified) clang-tools-extra/clang-doc/tool/CMakeLists.txt (+6-2)

diff --git a/clang-tools-extra/clang-doc/tool/CMakeLists.txt b/clang-tools-extra/clang-doc/tool/CMakeLists.txt
index e93a5728d6b6b..3fd41f187a617 100644
--- a/clang-tools-extra/clang-doc/tool/CMakeLists.txt
+++ b/clang-tools-extra/clang-doc/tool/CMakeLists.txt
@@ -25,7 +25,11 @@ set(assets
 )
 
 set(asset_dir "${CMAKE_CURRENT_SOURCE_DIR}/../assets")
-set(resource_dir "${CMAKE_BINARY_DIR}/share/clang-doc")
+if(MSVC)
+  set(resource_dir "${CMAKE_BINARY_DIR}/$<CONFIG>/share/clang-doc")
+else()
+  set(resource_dir "${CMAKE_BINARY_DIR}/share/clang-doc")
+endif()
 set(out_files)
 
 function(copy_files_to_dst src_dir dst_dir file)
@@ -52,4 +56,4 @@ add_custom_target(copy-clang-doc-assets
   COMMENT "Copying Clang-Doc Assets"
 )
 set_target_properties(copy-clang-doc-assets PROPERTIES FOLDER "Clang-Doc/Assets")
-add_dependencies(clang-doc copy-clang-doc-assets)
+add_dependencies(clang-doc copy-clang-doc-assets)
\ No newline at end of file

ilovepi · 2024-07-03T16:07:11Z

seems to fail on windows CI.

dyung

I believe (and the pre-merge testing seems to validate my belief) that this breaks the case where you build with ninja + Visual Studio. Also, I believe the Xcode generator is similar to the Visual Studio generator in that they both are multi-config generators, so that configuration is likely failing as well.

clang-tools-extra/clang-doc/tool/CMakeLists.txt

PeterChou1 · 2024-07-03T17:53:27Z

I believe (and the pre-merge testing seems to validate my belief) that this breaks the case where you build with ninja + Visual Studio. Also, I believe the Xcode generator is similar to the Visual Studio generator in that they both are multi-config generators, so that configuration is likely failing as well.

@ilovepi

I don't have access to a mac could you run build for checking clang-doc in xcode to see if it reports any errors?

ilovepi · 2024-07-03T19:10:29Z

I believe (and the pre-merge testing seems to validate my belief) that this breaks the case where you build with ninja + Visual Studio. Also, I believe the Xcode generator is similar to the Visual Studio generator in that they both are multi-config generators, so that configuration is likely failing as well.

@ilovepi

I don't have access to a mac could you run build for checking clang-doc in xcode to see if it reports any errors?

while I have access to a mac, I don't have the ability to use XCode due to company policy.

petrhosek · 2024-07-03T19:31:22Z

We already have set of CMake variables that we use to refer to bin or lib directory which account for various configurations:

llvm-project/llvm/CMakeLists.txt

Lines 446 to 467 in c02e8f7

    
           # They are used as destination of target generators. 
        
           set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin) 
        
           set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX}) 
        
           if(WIN32 OR CYGWIN) 
        
             # DLL platform -- put DLLs into bin. 
        
             set(LLVM_SHLIB_OUTPUT_INTDIR ${LLVM_RUNTIME_OUTPUT_INTDIR}) 
        
           else() 
        
             set(LLVM_SHLIB_OUTPUT_INTDIR ${LLVM_LIBRARY_OUTPUT_INTDIR}) 
        
           endif() 
        
           # Each of them corresponds to llvm-config's. 
        
           set(LLVM_TOOLS_BINARY_DIR ${LLVM_RUNTIME_OUTPUT_INTDIR}) # --bindir 
        
           set(LLVM_LIBRARY_DIR      ${LLVM_LIBRARY_OUTPUT_INTDIR}) # --libdir 
        
           set(LLVM_MAIN_SRC_DIR     ${CMAKE_CURRENT_SOURCE_DIR}  ) # --src-root 
        
           set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_SRC_DIR}/include ) # --includedir 
        
           set(LLVM_BINARY_DIR       ${CMAKE_CURRENT_BINARY_DIR}  ) # --prefix 
        
           # Note: LLVM_CMAKE_DIR does not include generated files 
        
           set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules) 
        
           set(LLVM_EXAMPLES_BINARY_DIR ${LLVM_BINARY_DIR}/examples) 
        
           set(LLVM_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)

I think we should introduce another one for the share directory and use that.

PeterChou1 · 2024-07-03T20:06:52Z

We already have set of CMake variables that we use to refer to bin or lib directory which account for various configurations:

llvm-project/llvm/CMakeLists.txt

Lines 446 to 467 in c02e8f7

# They are used as destination of target generators.

set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)

set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})

if(WIN32 OR CYGWIN)

# DLL platform -- put DLLs into bin.

set(LLVM_SHLIB_OUTPUT_INTDIR ${LLVM_RUNTIME_OUTPUT_INTDIR})

else()

set(LLVM_SHLIB_OUTPUT_INTDIR ${LLVM_LIBRARY_OUTPUT_INTDIR})

endif()

# Each of them corresponds to llvm-config's.

set(LLVM_TOOLS_BINARY_DIR ${LLVM_RUNTIME_OUTPUT_INTDIR}) # --bindir

set(LLVM_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}) # --libdir

set(LLVM_MAIN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR} ) # --src-root

set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_SRC_DIR}/include ) # --includedir

set(LLVM_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR} ) # --prefix

# Note: LLVM_CMAKE_DIR does not include generated files

set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules)

set(LLVM_EXAMPLES_BINARY_DIR ${LLVM_BINARY_DIR}/examples)

set(LLVM_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)

I think we should introduce another one for the share directory and use that.

I've introduced a variable in the root cmakelists.txt I'm not very familiar with cmake though I'm sure if this is the correct approach

PeterChou1 · 2024-07-03T20:52:50Z

@dyung I do not have access to mac for xcode could you provide me with a log of the build failure on xcode

dyung · 2024-07-03T21:29:18Z

@dyung I do not have access to mac for xcode could you provide me with a log of the build failure on xcode

% python3 ~/src/git/llvm-dyung/build/Debug/bin/llvm-lit -v basic-project.test
-- Testing: 1 tests, 1 workers --
FAIL: Clang Tools :: clang-doc/basic-project.test (1 of 1)
******************** TEST 'Clang Tools :: clang-doc/basic-project.test' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Emiting docs in html format.

--
Command Output (stderr):
--
RUN: at line 1: rm -rf /Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp && mkdir -p /Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs /Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build
+ rm -rf /Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp
+ mkdir -p /Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs /Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build
RUN: at line 2: sed 's|$test_dir|/Users/dyung/src/git/llvm-dyung/clang-tools-extra/test/clang-doc|g' /Users/dyung/src/git/llvm-dyung/clang-tools-extra/test/clang-doc/Inputs/basic-project/database_template.json > /Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build/compile_commands.json
+ sed 's|$test_dir|/Users/dyung/src/git/llvm-dyung/clang-tools-extra/test/clang-doc|g' /Users/dyung/src/git/llvm-dyung/clang-tools-extra/test/clang-doc/Inputs/basic-project/database_template.json
RUN: at line 3: clang-doc --format=html --output=/Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs --executor=all-TUs /Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build/compile_commands.json
+ clang-doc --format=html --output=/Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs --executor=all-TUs /Users/dyung/src/git/llvm-dyung/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build/compile_commands.json
default index.js file missing at /Users/dyung/src/git/llvm-dyung/build/Debug/bin/../share/clang-doc/index.js

This was run using the current HEAD, 4c63672.

PeterChou1 · 2024-07-04T01:35:30Z

I've added the install path using a platform agnostic variable LLVM_SHARE_OUTPUT_INTDIR which should fix build path problems

llvm-ci · 2024-07-05T19:56:11Z

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-darwin running on doug-worker-3 while building clang-tools-extra,llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/708

Here is the relevant piece of the build log for the reference:

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'lit :: shtest-external-shell-kill.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 23
env -u FILECHECK_OPTS "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/bin/python3.9" /Users/buildbot/buildbot-root/x86_64-darwin/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -a Inputs/shtest-external-shell-kill | grep -v 'bash.exe: warning: could not find /tmp, please create!' | FileCheck /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/utils/lit/tests/shtest-external-shell-kill.py
# executed command: env -u FILECHECK_OPTS /Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/bin/python3.9 /Users/buildbot/buildbot-root/x86_64-darwin/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -a Inputs/shtest-external-shell-kill
# note: command had no output on stdout or stderr
# error: command failed with exit status: 1
# executed command: grep -v 'bash.exe: warning: could not find /tmp, please create!'
# executed command: FileCheck /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/utils/lit/tests/shtest-external-shell-kill.py
# .---command stderr------------
# | /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/utils/lit/tests/shtest-external-shell-kill.py:29:15: error: CHECK-NEXT: is not on the line after the previous match
# | # CHECK-NEXT: end
# |               ^
# | <stdin>:22:22: note: 'next' match was here
# | RUN: at line 5: echo end
# |                      ^
# | <stdin>:8:6: note: previous match ended here
# | start
# |      ^
# | <stdin>:9:1: note: non-matching line after previous match is here
# | 
# | ^
# | 
# | Input file: <stdin>
# | Check file: /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/utils/lit/tests/shtest-external-shell-kill.py
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |          .
# |          .
# |          .
# |         17: RUN: at line 3: sleep 2 
# |         18: + sleep 2 
# |         19: + sleep 300 
# |         20: RUN: at line 4: kill $PID 
# |         21: + kill 50384 
# |         22: RUN: at line 5: echo end 
# | next:29                          !~~  error: match on wrong line
# |         23: /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/utils/lit/tests/Inputs/shtest-external-shell-kill/Output/test.txt.script: line 5: echo: write error: Interrupted system call 
# |         24: /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/utils/lit/tests/Inputs/shtest-external-shell-kill/Output/test.txt.script: line 6: 50384 Terminated: 15 sleep 300 
# |         25:  
# |         26: -- 
# |         27:  
# |          .
# |          .
...

mgorny · 2024-07-06T07:59:57Z

This change broke standalone clang builds against installed LLVM, since LLVM_SHARE_OUTPUT_INTDIR is not exported by LLVM and therefore resolves to an empty path:

ninja: error: mkdir(/clang-doc): Permission denied
ninja: build stopped: .

Why are you even declaring the directory inside LLVM when it's only used by Clang?

Please revert.

reverts #97540 which broke clangs standalone build

mgorny · 2024-07-10T06:50:55Z

Thank you.

[clang-doc] fix path bug introduced by asset test

b8dd4f6

PeterChou1 requested review from petrhosek, ilovepi and dyung July 3, 2024 08:45

llvmbot added the clang-tools-extra label Jul 3, 2024

PeterChou1 changed the title ~~[clang-doc] fix path bug introduced by asset test~~ [clang-doc] fix bug introduced by asset test Jul 3, 2024

dyung requested changes Jul 3, 2024

View reviewed changes

ilovepi reviewed Jul 3, 2024

View reviewed changes

clang-tools-extra/clang-doc/tool/CMakeLists.txt Outdated Show resolved Hide resolved

clang-tools-extra/clang-doc/tool/CMakeLists.txt Outdated Show resolved Hide resolved

[clang-doc] modify cmake file to fix asset install path

cf993cf

[clang-doc] fix nit

e389b78

[llvm] modify cmake list to add shared directory

4733341

[clang-doc] readd resource directory

3433bfe

PeterChou1 added 2 commits July 3, 2024 18:47

[clang-doc] use LLVM_RUNTIME_OUTPUT_INTDIR

8819740

[llvm] fix version

50bc61f

PeterChou1 requested review from ilovepi and dyung July 3, 2024 23:04

PeterChou1 added 3 commits July 4, 2024 00:35

[llvm] introduce new cmake global variable LLVM_SHARE_OUTPUT_INTDIR

0f63fd2

[clang-doc] fix syntax error

f364f41

Merge branch 'main' into clang-doc-fix-path-bug

fddc9d3

ilovepi approved these changes Jul 5, 2024

View reviewed changes

PeterChou1 merged commit 1ed84a8 into llvm:main Jul 5, 2024
7 checks passed

kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024

[clang-doc] fix bug introduced by asset test (llvm#97540)

496d2e1

PeterChou1 mentioned this pull request Jul 6, 2024

[clang-doc] revert asset bug fix #97882

Merged

PeterChou1 added a commit that referenced this pull request Jul 6, 2024

[clang-doc] revert asset bug fix (#97882)

ac9d34a

reverts #97540 which broke clangs standalone build

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang-doc] fix bug introduced by asset test #97540

[clang-doc] fix bug introduced by asset test #97540

Uh oh!

PeterChou1 commented Jul 3, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jul 3, 2024

Uh oh!

ilovepi commented Jul 3, 2024

Uh oh!

dyung left a comment

Uh oh!

Uh oh!

Uh oh!

PeterChou1 commented Jul 3, 2024

Uh oh!

ilovepi commented Jul 3, 2024

Uh oh!

petrhosek commented Jul 3, 2024

Uh oh!

PeterChou1 commented Jul 3, 2024

Uh oh!

PeterChou1 commented Jul 3, 2024

Uh oh!

dyung commented Jul 3, 2024 •

edited

Loading

Uh oh!

PeterChou1 commented Jul 4, 2024 •

edited

Loading

Uh oh!

Uh oh!

llvm-ci commented Jul 5, 2024

Uh oh!

mgorny commented Jul 6, 2024

Uh oh!

mgorny commented Jul 10, 2024

Uh oh!

Uh oh!

[clang-doc] fix bug introduced by asset test #97540

[clang-doc] fix bug introduced by asset test #97540

Uh oh!

Conversation

PeterChou1 commented Jul 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 3, 2024

Uh oh!

ilovepi commented Jul 3, 2024

Uh oh!

dyung left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

PeterChou1 commented Jul 3, 2024

Uh oh!

ilovepi commented Jul 3, 2024

Uh oh!

petrhosek commented Jul 3, 2024

Uh oh!

PeterChou1 commented Jul 3, 2024

Uh oh!

PeterChou1 commented Jul 3, 2024

Uh oh!

dyung commented Jul 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PeterChou1 commented Jul 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

llvm-ci commented Jul 5, 2024

Uh oh!

mgorny commented Jul 6, 2024

Uh oh!

mgorny commented Jul 10, 2024

Uh oh!

Uh oh!

PeterChou1 commented Jul 3, 2024 •

edited

Loading

dyung commented Jul 3, 2024 •

edited

Loading

PeterChou1 commented Jul 4, 2024 •

edited

Loading