Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[question] packaging debug visualizers (natvis) #16916

Closed
1 task
vasama opened this issue Aug 31, 2024 · 46 comments · Fixed by #17214
Closed
1 task

[question] packaging debug visualizers (natvis) #16916

vasama opened this issue Aug 31, 2024 · 46 comments · Fixed by #17214
Assignees
Milestone

Comments

@vasama
Copy link

vasama commented Aug 31, 2024

What is your question?

I want to package a library containing Visual Studio natvis debug visualizers for its interface types. I could not find any documentation on how to approach this with Conan. This requires adding the natvis files as sources on the CMake target generated by CMakeDeps. Is there an existing way to achieve this in the package recipe?

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded memsharded self-assigned this Aug 31, 2024
@memsharded
Copy link
Member

Hi @vasama

Thanks for your question.

This requires adding the natvis files as sources on the CMake target generated by CMakeDeps. Is there an existing way to achieve this in the package recipe?

At the moment there is not built-in direct way to do this.
The packages can define in package_info() a self.cpp_info.srcdirs to indicate the consumers there are some source files there, but these files are not automatically appended to targets. The usual approach is that consumers know what to do with these files. Can you please elaborate a bit more how do you add sources to a CMake target and how the consumers will automatically use (or not) these sources?

I think the best approach could be to generate a CMake build module that manages to add those extra files to the targets, see for example https://docs.conan.io/2/examples/graph/tool_requires/use_cmake_modules.html. These modules are automatically included at the end of find_package() xxxx-config.cmake script, so they execute and could add information to targets.

@vasama
Copy link
Author

vasama commented Sep 1, 2024

Adding the .natvis file as an INTERFACE source on an IMPORTED INTERFACE library allows Visual Studio to see the visualizer in a dependent project:

cmake_minimum_required(VERSION 3.24)
project(test)

add_library(lib INTERFACE IMPORTED)
target_sources(lib INTERFACE lib.natvis)

add_executable(app app.cpp)
target_link_libraries(app PRIVATE lib)

@memsharded
Copy link
Member

target_sources(lib INTERFACE lib.natvis)

But what is exactly the usage of these files in the consumer project? Are they compiled automatically in any consumer target that does a target_link_libraries() over the imported target that is created with find_package()? Or is it only for visualizing the file in the IDE?

@vasama
Copy link
Author

vasama commented Sep 2, 2024

The IDE uses them for debug visualisation and I believe may also embed them into .pdb files.

@vasama
Copy link
Author

vasama commented Sep 2, 2024

Having tested it now, MSVC does indeed embed them in the .pdb files generated for consumer projects.

@memsharded
Copy link
Member

Thanks for the feedback.

Yes, I think the best approach could be to define a cmake_build_module that add the sources you want to the target.
Besides the docs, I think this test here can be useful:

, it is a full example of adding information to the CMake targets (in the test case an alias) and how it is automatically injected to the consumers, I think the same approach can be used for adding sources. Please give this a try and let us know.

@vasama
Copy link
Author

vasama commented Sep 6, 2024

Okay, I made it work using a CMake build module. During the CMake configuration of the package, I generate the build module script at build/cmake_build_modules/my_script.cmake, where build is the CMake project binary directory. When the project is installed, that file is copied into package/cmake_build_modules/my_script.cmake. package_info finds all the CMake scripts in the package/cmake_build_modules/ directory and sets the cpp_info cmake_build_modules property accordingly. This works when the package is created, but not when the package is in editable mode. It's not clear to me how I can make package_info find the scripts when in editable mode.

@memsharded
Copy link
Member

Great, happy that you were able to make it work.

This works when the package is created, but not when the package is in editable mode. It's not clear to me how I can make package_info find the scripts when in editable mode.

For the editable case, the location of the module at "build" time should be defined in the layout() method, something like:

def layout(self):
     ...
     self.cpp.source.builddirs = ["build/cmake_build_modules"]

@vasama
Copy link
Author

vasama commented Sep 7, 2024

Shouldn't that instead be:

def layout(self):
	...
	self.cpp.build.builddirs = ["cmake_build_modules"]

In any case, that still leaves detecting the build modules. Currently I do this to find them in the package folder:

def package_info(self):
	...

	# Find and add all scripts in package_folder/cmake_build_modules/
	cmake_build_modules_dir = os.path.join(self.package_folder, "cmake_build_modules")
	if os.path.isdir(cmake_build_modules_dir):
		cmake_build_modules = self.cpp_info.get_property("cmake_build_modules") or []

		for entry in os.scandir(cmake_build_modules_dir):
			cmake_build_modules.append(os.path.join(cmake_build_modules_dir, entry.name))

		if len(cmake_build_modules) > 0:
			self.cpp_info.set_property("cmake_build_modules", cmake_build_modules)

It's still not clear to me how I can make this work in editable mode.

@memsharded
Copy link
Member

self.cpp.build.builddirs

It depends. If the modules are generated at build time, it is self.cpp.build. If they are already part of the source, it is self.cpp.source.

I think the issue is that the folder is relative to the package folder, not to the build-folder, this is why I used self.cpp.source.builddirs = ["build/cmake_build_modules"], with the build/ prefix. Did you try it?

cmake_build_modules = self.cpp_info.get_property("cmake_build_modules") or []

Not very clear why you are doing it, the cpp_info.get_property() should always be empty, isn't it? This is your package property, so not anyone else would be setting it.

It's still not clear to me how I can make this work in editable mode.

Maybe the best would be to try to put this in a full reproducible example, either a repo with some minimalistic code if you think you could easily provide it, or I can try to provide a test in the Conan test suite later.

@memsharded memsharded added this to the 2.8.0 milestone Sep 7, 2024
@vasama
Copy link
Author

vasama commented Sep 7, 2024

I have made a small example here: https://github.com/vasama/conan-issue-16916

cd ./lib
conan create .

cd ../app
conan install .
cmake --preset $PRESET # My Conan profile produces an MSBuild solution
cmake --open $BUILDDIR # Open the solution in Visual Studio

Note the .natvis file in the app project:
Screenshot 2024-09-07 115532

Note the visualization size=5 defined in the natvis file.
Screenshot 2024-09-07 120340

@memsharded
Copy link
Member

Hi @vasama

Thanks very much for putting together the code, that really helped.

I'd like to apologize, because I was mistaken and I gave confusing information. I was mixing the self.cpp.xxx.builddirs = ["cmake_build_modules"] with the set_property("cmake_build_modules"), which aim for a similar purpose, but not what you were looking for in this case. The right approach is using set_property().

I have done a PR in vasama/conan-issue-16916#1, I'll comment the details there

@memsharded
Copy link
Member

I tested both flows, both with conan create and also with the conan editable add lib

@vasama
Copy link
Author

vasama commented Sep 13, 2024

I made a related issue on the CMake issue tracker here, since the CMake support around .natvis files could use some improvement as well.

Since you added this to the 2.8.0 milestone, are you planning some Conan changes to address this?

@memsharded
Copy link
Member

No, I don't think it is needed any change in Conan client at the moment. I just wanted to make sure that this ticket wasn't buried, so assigning it to 2.8 was the way. I think we might close the ticket now? Thanks for the feedback!

@vasama
Copy link
Author

vasama commented Sep 17, 2024

Actually, I still couldn't make it work. When the script is generated in the build directory, I'm not sure how I can specify it in the cmake_build_modules in package_info.

ConanException: 'self.build_folder' access in 'package_info()' method is forbidden

@memsharded
Copy link
Member

memsharded commented Sep 18, 2024

ConanException: 'self.build_folder' access in 'package_info()' method is forbidden

This is expected, the package_info() should operate only on the self.package_folder, because it must work even when the precompiled package has been downloaded from the server and not built locally, when the build_folder doesn't even exist locally.

Still I am not sure if I follow, maybe it is necessary to update the repo with the code? Because there I don't see the self.build_folder usage in the package_info() method. The PR vasama/conan-issue-16916#1 is still open, so maybe merging it, or closing it, but updating the code could help to understand it.

@vasama
Copy link
Author

vasama commented Sep 18, 2024

Yes, in the example it uses package_folder, which works because the file is copied from the build folder to the package folder during package(). Naturally this does not work in editable mode because there is no call to package(). Before I can even worry about relative paths of the CMake build module and the visualizer, first I have to actually specify the build module in package_info(). In editable mode, when it's in the build folder, I don't know how to do that.

@vasama
Copy link
Author

vasama commented Sep 18, 2024

I had another look at your previous comments, and I see that I missed this part:

def layout(self):
    ...
    self.cpp.build.set_property("cmake_build_modules", ["my_cmake_build_module.cmake"])

This gets a little closer, but I can't do it unconditionally, because whether the file actually exists is never checked. It seems to me that I also cannot check in layout() whether it does exist.

@vasama
Copy link
Author

vasama commented Sep 18, 2024

It seems to me that the CMake build module concept would be a lot more usable if

  1. one could specify either directories (like builddirs) from which all .cmake files are included, or if CMake build modules that don't exist were ignored, and
  2. before including the file, the script generated by CMakeDeps set some variable like CONAN_CURRENT_PACKAGE_DIR that could be used to find files from the current package.

@memsharded memsharded modified the milestones: 2.8.0, 2.9.0 Sep 27, 2024
@memsharded
Copy link
Member

Hi @vasama

I am resuming having a look to your suggestions while developing a new CMakeDeps integration, see #16964

However, it is still not fully clear what would be the pain being solved. I think that what you are requesting can be easily solved with a one liner thanks to python:

def package_info(self):
    self.cpp_info.set_property("cmake_build_modules", glob.glob("myfolder/*.cmake"))

That will pick all cmake modules in your specified folder, and it will not fail because of missing modules because it will only pick the modules that actually exist.

Then, these files doesn't need to be include(), they are automatically loaded and available, so I am not sure I fully understand your last point 2.

@vasama
Copy link
Author

vasama commented Oct 24, 2024

I think the reason I couldn't do the glob as you suggest, was that the build modules are generated by CMake in the build directory, where they remain when the package is consumed in editable mode, and Conan didn't allow me to use the build directory during package_info.

As for using package files from the build module, in this case I mean the .natvis files themselves. I don't know how to find them in the build module when they are located in the source directory in editable mode, or the package directory in regular mode, or even in the binary directory in editable mode if I'm using some sort of templating to generate them during build.

@vasama
Copy link
Author

vasama commented Oct 24, 2024

To support what I want to do, I think we can simplify this to the following requirements:

  1. CMake build generates zero or more build modules, and it must be possible to automate their inclusion in Conan.
  2. CMake build generates a text file, and it must be possible to print its contents from a build module during CMake configuration of a consumer.

Obviously I'm interested in more than just printing the contents, but I think that's a reasonable simplification for now.

@memsharded
Copy link
Member

Regarding point 1 and the editable thing, I am still trying to figure out what are the issues and current possibilities. Wouldn't something like this work?:

def layout(self):
    self.cpp.source.set_property("cmake_build_modules", glob.glob(os.path.join(self.source_folder, "mysrcmodules/*.cmake"))
    self.cpp.build.set_property("cmake_build_modules", glob.glob(os.path.join(self.build_folder, "mybuiltmodules/*.cmake"))

def package_info(self):
    self.cpp_info.set_property("cmake_build_modules", glob.glob("myfolder/*.cmake"))

Basically, there are 3 different sources that could contain build modules, in different states (editable, final package), so these 3 folders (mysrcmodules, mybuiltmodules, myfolder) needs to be specified to Conan somehow, I think the above might be a possibility to implement it, or am I still missing something?

I am also not saying that this couldn't improve the UX, but it is important to understand if/why this is possible or not, and what are the reasons. Thanks for the feedback!

@vasama
Copy link
Author

vasama commented Oct 24, 2024

It looks like self.source_folder and self.build_folder are None when layout is called.

@memsharded
Copy link
Member

It looks like self.source_folder and self.build_folder are None when layout is called.

Uhmm, good point. Let me try to come up with a working test.

@memsharded
Copy link
Member

I am doing this test in this PR, please have a look: #17214

It is true that there is a small bug there that doesn't allow to use simultaneously cpp.source.set_property("cmake_build_modules") and cpp.build.set_property("cmake_build_modules") in the layout() because one will overwrite the other (it wouldn't be a blocker, it is mostly a conceptual distinction in practice all modules can be assigned to one of them). But with the PR solving also this issue, it seems that it is working, both "source" and dynamically created "build" cmake-build-modules can be used both as editable and from inside the package.

If you could please have a look at the test, let me know if this would be capturing your needs or still something would be missing.

@vasama
Copy link
Author

vasama commented Oct 24, 2024

To clarify, does that PR allow using self.source_folder and self.build_folder in layout? In that case, I suppose it solves the first requirement, but that still leaves the second.

@memsharded
Copy link
Member

o clarify, does that PR allow using self.source_folder and self.build_folder in layout? In that case, I suppose it solves the first requirement, but that still leaves the second.

No, it needs to compose the path with os.path.join(self.recipe_folder, self.folders.build, "mypath"), but the result would be equivalent to the conceptual self.build_folder, yes.

I think I can add some extra checks to the test to implement the case 2)

@vasama
Copy link
Author

vasama commented Oct 24, 2024

it needs to compose the path

Okay, I looked at the test more closely. How does this work with cmake_layout and something like tools.cmake.cmake_layout:build_folder_vars+=["settings.compiler"] that causes the build directory to be located at e.g. $PROJECT/build/msvc when using MSVC?

@memsharded
Copy link
Member

I am finding a limitation in CMake. It doesn't allow to read a .txt (or other) file relative to the current .cmake file. All variables are related to the CMakeLists.txt list file, but not to the current .cmake. That means that a CMake module file doesn't have a way to file(READ <somepath> myvar) and load a file that is just besides the .cmake module.

Conan can implement this with:

  • at build-time use the self.build_folder as hardcoded absolute path for <somepath>
  • at package() rewrite the file with the new package folder absolute path replacing the previous build folder one
  • Use the finalize() new method to make this package relocatable and re-write the .cmake build module with an absolute path pointing to the final installed location

Doesn't seem trivial, but I don't see any other way that this can be done, unless you know some other way to overcome this CMake limitation.

@vasama
Copy link
Author

vasama commented Oct 24, 2024

Did you try CMAKE_CURRENT_LIST_DIR?

@memsharded
Copy link
Member

Yes, I did. But that will get the value of the app/CMakeLists.txt of the consumer, because that is the current LIST file 😞

Same with other CMAKE_ variables that I have checked, there is none that I have found that refers to the path where a .cmake file that is a module that is "include()" included.

@vasama
Copy link
Author

vasama commented Oct 24, 2024

# C:/Users/vasama/sandbox/conan-consumer/CMakeLists.txt
cmake_minimum_required(VERSION 3.24)
project(conan-consumer)
include(cmake/test.cmake)
# C:/Users/vasama/sandbox/conan-consumer/cmake/test.cmake
message(STATUS "CMAKE_CURRENT_LIST_DIR='${CMAKE_CURRENT_LIST_DIR}'")

For me this produces:

C:\Users\vasama\sandbox\conan-consumer λ cmake -B build/msvc
-- Using Conan toolchain: C:/Users/vasama/sandbox/conan-consumer/build/msvc/generators/conan_toolchain.cmake
-- Conan toolchain: CMAKE_GENERATOR_TOOLSET=v143
-- Conan toolchain: Setting CMAKE_MSVC_RUNTIME_LIBRARY=$<$<CONFIG:Debug>:MultiThreadedDebugDLL>
-- Conan toolchain: C++ Standard 23 with extensions OFF
-- Selecting Windows SDK version 10.0.22621.0 to target Windows 10.0.22631.
-- CMAKE_CURRENT_LIST_DIR='C:/Users/vasama/sandbox/conan-consumer/cmake'
-- Configuring done (0.0s)
-- Generating done (0.0s)
-- Build files have been written to: C:/Users/vasama/sandbox/conan-consumer/build/msvc

@memsharded
Copy link
Member

Thanks for the hint. I realized what is happening. The variable seem to have different scope when evaluated inside a function(), which is what I was doing. I'll try with a macro or using global var.

@vasama
Copy link
Author

vasama commented Oct 24, 2024

CMake 3.17 has CMAKE_CURRENT_FUNCTION_LIST_DIR etc. I suppose in earlier versions you can define a global variable in the same file where the function is defined.

@memsharded
Copy link
Member

Ok, even when using macros, the scope of the variable change. I managed to make it work with:

set(MY_CMAKE_PATH ${CMAKE_CURRENT_LIST_DIR}
macro(otherfunc)
   file(READ "${MY_CMAKE_PATH}/my.txt" c)
   message("Hello ${c}!!!!")
endmacro()

It is included in the PR test now

@vasama
Copy link
Author

vasama commented Oct 24, 2024

it needs to compose the path

Okay, I looked at the test more closely. How does this work with cmake_layout and something like tools.cmake.cmake_layout:build_folder_vars+=["settings.compiler"] that causes the build directory to be located at e.g. $PROJECT/build/msvc when using MSVC?

I still don't see how this is going to work though.

@memsharded
Copy link
Member

Added cmake_layout() and tools.cmake.cmake_layout:build_folder_vars to the test.

@vasama
Copy link
Author

vasama commented Oct 25, 2024

Okay. That works for the case when the .natvis (.txt) file is generated by the CMake build. I should have specified the original case of using a source file in my requirements:

  1. It must be possible to print the contents of a source file in the package/recipe folder (depending on mode) from a build module during CMake configuration of a consumer.

I think this is not yet possible? I'm not sure how I could find the appropriate path from the build module script. You mentioned the finalize method. I suppose that might work, but it seems a little suspect.

What I suggested earlier was to specify some CMake variables before including the build module. Perhaps something like CONAN_CURRENT_PACKAGE_DIR that points to either the package directory or the recipe directory depending on mode.

@memsharded
Copy link
Member

I have added a new test that uses some fake vis/my.vis files inside the source folder.
Using the

set(MY_CMAKE_PATH ${CMAKE_CURRENT_LIST_DIR})
macro(otherfunc)
file(READ "${MY_CMAKE_PATH}/my.vis" c)

also works, taking into account:

  • For the final package() it is easy to put the vis files in the right location besides the CMake module
  • For the editable layout is is possible to just do a copy of those vis files to the build folder, something like:
class Conan(ConanFile):
            name = "myfunctions"
            version = "1.0"
            exports_sources = ["src/*.cmake", "src/vis/*.vis"]
            settings = "build_type", "arch"

            def build(self):
                copy(self, "*.vis", src=self.source_folder, dst=self.build_folder, keep_path=False)

And that will allow the local build-module to find the vis files too.

@vasama
Copy link
Author

vasama commented Oct 26, 2024

  • For the editable layout is is possible to just do a copy of those vis files to the build folder, something like:

I considered copying, but it somewhat defeats the point of using an editable layout, because changes in the source file are not immediately reflected in consumers. One might suggest using a symbolic link instead, but at least on Windows that's not a reasonable solution due to the administrative permissions required for creating one.

@memsharded
Copy link
Member

memsharded commented Oct 28, 2024

That is fine, I am not very happy about the copy either. But it is important for us to analyze the alternatives to decide and prioritize, it is not the same something that cannot be implemented by any mean, that something that as some solution that something that doesn't have a great solution but has a reasonable workaround

In this case, the changes in source, in most cases aren't directly visible to the consumers of the editable packages, editable packages still need to be "built" and build those sources into the form that the consumers can use. This is why the --build=editable was added, to simplify this task. If the .vis files are not huge, then I would consider this approach as a reasonable workaround.

I have been trying to improve this, based on your suggestion of a variable such as CONAN_CURRENT_PACKAGE_DIR.
The thing is that we already have a variable for this, I have been trying to add it in the test, it is called for this example CONAN_CURRENT_PACKAGE_DIR. So I tried:

set(MY_CMAKE_PATH ${myfunctions_PACKAGE_FOLDER_RELEASE})
macro(otherfunc)
file(READ "${MY_CMAKE_PATH}/src/vis/my.vis" c)
'message("Hello ${c}!!!!")\nendmacro()

The myfunctions_PACKAGE_FOLDER_RELEASE in editable mode is pointing to the "root" of the project. So to make it work for editable without an extra copy, I needed to do ${MY_CMAKE_PATH}/src/vis/my.vis.

But now, when this is in the real package, this path will no longer be valid. I can solve it by re-defining the module in the package() method like:

     def package(self):
                copy(self, "*.cmake", self.source_folder, os.path.join(self.package_folder, "mods"),
                     keep_path=False)
                copy(self, "*.vis", self.source_folder, os.path.join(self.package_folder, "mods"),
                     keep_path=False)
                cmake = 'set(MY_CMAKE_PATH ${myfunctions_PACKAGE_FOLDER_RELEASE})\n'\
                        'macro(otherfunc)\n'\
                        'file(READ "${MY_CMAKE_PATH}/mods/my.vis" c)\n'\
                        'message("Hello ${c}!!!!")\nendmacro()'
                save(self, os.path.join(self.package_folder, "mods/otherfuncs.cmake"), cmake)

I updated the test accordingly in commit: a79dc44

BUT THEN, I realized that these natvis files would probably be a good model for the cpp_info.resdirs, so I changed the test to use it:

  • Define self.cpp.source.resdirs = ["vis"] in the layout() method, for editable files in the source folder
  • Define self.cpp_info.resdirs = ["mods"] in the package_info() method, for the packaged case
  • Use set(MY_CMAKE_PATH ${myfunctions_RES_DIRS_RELEASE}) in the cmake-module to point to that folder.

It seems to work nicely. Please have a look to the latest PR test (the second test), and let me know what you think.

@memsharded memsharded modified the milestones: 2.9.0, 2.10.0 Oct 28, 2024
@vasama
Copy link
Author

vasama commented Oct 28, 2024

From looking at the files generated by CMakeDeps I was familiar with the myfunctions_PACKAGE_FOLDER_RELEASE variables, but it's problematic because it requires knowing both the package name and config, or perhaps even the values of multiple package options? The code generated by CMakeDeps knows those things when it performs the build module include, and communicating them to the build module in some standardised fashion would make build modules much more powerful. It's not just directory paths either, but also for example the names of the targets generated by CMakeDeps that would be useful.

@memsharded
Copy link
Member

Both the package name and config are available as self.name and self.settings.build_type, updated test:

config = str(self.settings.build_type).upper()
cmake = f'set(MY_CMAKE_PATH ${{{self.name}_RES_DIRS_{config}}})\n'\

The problem is that it is not the PACKAGE_FOLDER that needs to be used there, but another folder, in this case the RES_DIRS. Because we found that it kind of makes sense that this files are "resource files", but this depends a bit on the specific use case, other users might want to use the includedirs or libdirs for other purposes?

@memsharded
Copy link
Member

Hi @vasama

This ticket has been closed by PR #17214 that contained the fix for the cmake_build_modules to work nicely in editable.

Please let us know if there is any further question or issue, feel free to re-open or create new ticket.

As a side note, I have verified that the variable myfunctions_PACKAGE_FOLDER_RELEASE also exists in the new CMakeDeps generator that will eventually replace the existing one, and it was release in Conan 2.9 for dev-testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants