[workspace] [POC] Handle all the packages involved in a WS (editables and dependents)

[jgsogo]

Changelog: (Feature | Fix | Bugfix): Describe here your pull request
Docs: https://github.com/conan-io/docs/pull/XXXX

Related to #5291 (close?) and #4879 (superseedes)

This PR implements a new approach of workspaces: packages declared inside the conanws.yml file are consumed using CMake targets (see (1) in the TODO list), and packages that depend on any of these are declared as custom_targets (only a sentinel at this moment, see (2) in the TODO list). CMake models all the dependencies between these projects and triggers the build of the projects that are affected by any change.

Graph used in the tests:

In this implementation, we can distinguish three categories for packages:

1. packages included in the workspace, those declared as editable in the conanws.yml: pkgA, pkgB, pkgE and pkgG
2. packages not included in the workspace, and without any dependency to packages included in the workspace: pkgH, pkgD
3. packages not included in the workspace that depend on packages included in the workspace: pkgC and pkgF

Workspace creation

We need the conanws.yml file:

editables:
    pkgA/0.1@ws/testing:
        path: /private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmp2n7z7h55conans/pathwithoutspaces/pkgA
        layout: /private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmp2n7z7h55conans/pathwithoutspaces/pkgA/layout
    pkgB/0.1@ws/testing:
        path: /private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmpi2uxtn3cconans/pathwithoutspaces/pkgB
        layout: /private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmpi2uxtn3cconans/pathwithoutspaces/pkgB/layout
    pkgE/0.1@ws/testing:
        path: /private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmphy2up924conans/pathwithoutspaces/pkgE
        layout: /private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmphy2up924conans/pathwithoutspaces/pkgE/layout
    pkgG/0.1@ws/testing:
        path: /private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmp833gi6xhconans/pathwithoutspaces/pkgG
        layout: /private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmp833gi6xhconans/pathwithoutspaces/pkgG/layout
workspace_generator: cmake

Comments about the fields:

• we don't need the roots, all the editables will be included as roots so we get sure they override any existing dependency upstreams. This solves the issue of the version ranges.
• workspace_generator: currently only cmake, but in the future it should accept visual_studio too.
• There is no need for generators associated to the editables, the generators needed for each package should be already in its recipe.
• All of the editables can use the same layout file, as it was done before.

Generated files (CMake)

CMakeLists.txt

I'm generating the CMakeLists.txt file:

cmake_minimum_required(VERSION 3.3)
project("Conan Workspace")

include(${CMAKE_CURRENT_SOURCE_DIR}/conanbuildinfo.cmake)
conan_basic_setup(NO_OUTPUT_DIRS)  # Execute Conan magic

set(CMAKE_SKIP_RPATH 0)  # We are not creating packages here, it is ok to have rpaths

include(${CMAKE_CURRENT_SOURCE_DIR}/conanworkspace.cmake)

• I can have a name for the project(...) if I add a name field to conanws.yml file
• I'm including a conanbuildinfo.cmake to keep doing Conan magic, this file has been generated without dependencies.
• As the workspace is not going to create a package, I can use RPATHS, so linux binaries will find shared libraries. ⚠️ Need to check if I can pass SKIP_RPATHS for the same behavior.
• I need NO_OUTPUT_DIRS so each library will generate the binaries in the place defined by the layout file (see (3) in TODO list).

conanworkspace.cmake

And also, it generates a conanworkspace.cmake file, I'm dividing it into sections to talk about each one:

# List of targets involved in the workspace
    list(APPEND ws_targets "pkgD")
    list(APPEND ws_targets "pkgH")
    list(APPEND ws_targets "pkgA")
    list(APPEND ws_targets "pkgC")
    list(APPEND ws_targets "pkgB")
    list(APPEND ws_targets "pkgF")
    list(APPEND ws_targets "pkgE")
    list(APPEND ws_targets "pkgG")

    # Packages that are consumed by editable ones
        find_package(pkgD REQUIRED)
        set_target_properties(pkgD::pkgD PROPERTIES IMPORTED_GLOBAL TRUE)
        add_library(CONAN_PKG::pkgD ALIAS pkgD::pkgD)
    
        find_package(pkgH REQUIRED)
        set_target_properties(pkgH::pkgH PROPERTIES IMPORTED_GLOBAL TRUE)
        add_library(CONAN_PKG::pkgH ALIAS pkgH::pkgH)
    
        find_package(pkgC REQUIRED)
        set_target_properties(pkgC::pkgC PROPERTIES IMPORTED_GLOBAL TRUE)
        add_library(CONAN_PKG::pkgC ALIAS pkgC::pkgC)
    
        find_package(pkgF REQUIRED)
        set_target_properties(pkgF::pkgF PROPERTIES IMPORTED_GLOBAL TRUE)
        add_library(CONAN_PKG::pkgF ALIAS pkgF::pkgF)

These packages are consumed by editable ones, I cannot know if they are being consumed using the cmake + TARGET approach or the cmake_find_package approach, so I'm generating the alias for both.

• ⚠️ pkgC and pkgF must be creating transitive targets too, check how CMake handles this duplication.
• ⚠️ I can traverse the recipes declared in conanws.yml to check which generators are being used.

# Override functions to avoid importing their own TARGETs (or calling again Conan Magic)
function(conan_basic_setup)
    message("Ignored call to 'conan_basic_setup'")
endfunction()

function(include)
    if ("${ARGV0}" MATCHES ".*/conanbuildinfo(_multi)?.cmake")
        message("Ignore inclusion of ${ARGV0}")
    else()
        _include(${ARGV})
    endif()
endfunction(include)

Override functions, I have already executed conan_basic_setup, no need to do it again (also, I don't want ${CONAN_LIBS} to be populated). ⚠️ I think it is ok if we do not support consuming dependencies using ${CONAN_LIBS}

# Do not use find_package for those packages handled within the workspace
function(find_package)
    if(NOT "${ARGV0}" IN_LIST ws_targets)
        # Note.- If it's been already overridden, it will recurse forever
        message("find_package(${ARGV0})")
        _find_package(${ARGV})
    else()
        message("find_package(${ARGV0}) ignored, it is a target handled by Conan workspace")
    endif()
endfunction()

I don't want to find_package again any library that is being handled by the workspace.

# Custom target
function(outer_package PKG_NAME FULL_REF)
    set(PKG_SENTINEL "/private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmpi6nniq7oconans/path with spaces/${PKG_NAME}.setinel")
    add_custom_command(OUTPUT ${PKG_SENTINEL}
                       COMMAND echo "Package ${FULL_REF} not built" > ${PKG_SENTINEL}
                       WORKING_DIRECTORY "/private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmpi6nniq7oconans/path with spaces"
                       COMMENT "Build ${PKG_NAME} outside workspace")
    add_custom_target(${PKG_NAME} DEPENDS ${PKG_SENTINEL})
endfunction()

Custom targets for package corresponding to the category (3), right now this is just a custom_target running a dummy custom_command (see (2) in the TODO list).

        # Inner: pkgA/0.1@ws/testing
        add_subdirectory("/private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmp2n7z7h55conans/pathwithoutspaces/pkgA" "/private/var/folders/yq/14hmvxm96xd7gfgl37_tnrbh0000gn/T/tmp2n7z7h55conans/pathwithoutspaces/pkgA/build")
        add_library(pkgA::pkgA ALIAS pkgA)
        add_library(CONAN_PKG::pkgA ALIAS pkgA)

Packages inside the workspace (category (1)) are included using add_subdirectory. Afterwards, I need to create the aliases corresponding to generators cmake and cmake_find_package because these packages can be consumed using any of those approaches.

        # Outter: pkgC/0.1@ws/testing
        outer_package(pkgC pkgC/0.1@ws/testing)
        add_dependencies(pkgC pkgA)
        add_dependencies(pkgC pkgD)

Packages outside the workspace are added as custom_target with the dependencies declared. This is needed to have the full graph representation of the workspace into the CMake model.

TODO:

1. Currently target name must match the name of the package, there are a couple of this to improve here:
   • The conanws.yml can contain a target field to use. But still, this is limited to only one target per package.
   • Ongoing work on Extended cpp_info model object to new components concept #5242 can be used to know all the targets included in the package an to generate the corresponding aliases.
2. Packages outside the workspace don't take into account changes of editable packages. This can be implemented, taken advantage of the graphlock feature, but I can only imagine a scenario where Conan is being run from inside CMake.
3. Where to generate the binaries?
   • package inside the workspace should generate the binaries in the workspace folder (and it is easy), BUT these packages will be found by packages not in the workspace using the editable feature... ⚠️ Building packages outside the workspace is not considered yet, so we can move the generation of this binaries to the workspace folder.
   • packages outside the workspace are not being generated.

Other tasks ⚠️

• Check problems related to paths ([workspaces] Cannot run apps which depend on editable shared libs #5132)
• Check with unconnected graphs
• Check Debug/Relase generators
• Verify that the implementation could be valid for others than cmake generator

@tags: slow

[jgsogo]

Having seen all these flags, it comes to my mind the idea of creating a conanfile.py to use build helpers to build/handle the workspace... 🤔

[Lawrencemm]

Does this by any chance make it possible in future to have packages in a workspace that aren't actually being built by cmake itself? That would be cool.

I'm generating the CMakeLists.txt file

I'd prefer to be including some generated cmake file only, not having the main one generated for me.
It used to be the way you have it here, there was an issue where people complained about it and it was changed to my suggestion. I guess if we must go this route we should probably reopen that issue to explain why we must do it this way.

include(${CMAKE_CURRENT_SOURCE_DIR}/conanworkspace.cmake)

Do we need to have it in the source directory? Would prefer my source directories to be untouched by generated stuff.

Are layouts still necessary in this model? Isn't it the case that packages can put their editable layout in the conanfile? Would be cool if we only needed to specify layouts for those packages that don't have it defined in their recipe.

[jgsogo]

Does this by any chance make it possible in future to have packages in a workspace that aren't actually being built by cmake itself? That would be cool.

Yes, now that we have lockfiles it is fairly easy to create a custom CMake target to run conan build <ref> -l workspace.lockfile under the hood. It would be nice to have a way to run build on a package in the cache (and even nicer to use a local build folder instead of the cache one) but right now an install --create could be triggered.

---

I'm generating the CMakeLists.txt file

I'd prefer to be including some generated cmake file only, not having the main one generated for me.
It used to be the way you have it here, there was an issue where people complained about it and it was changed to my suggestion. I guess if we must go this route we should probably reopen that issue to explain why we must do it this way.

Probably this is just a detail, Conan may autogenerate a CMakeLists.txt file if it is not found in the install-folder, or we can leave it to the user. Something to take into account, but we need to agree before on the core concepts of this PR.

---

include(${CMAKE_CURRENT_SOURCE_DIR}/conanworkspace.cmake)

Do we need to have it in the source directory? Would prefer my source directories to be untouched by generated stuff.

I don't think this is necessary, it can probably be in any folder. Again, I think this is something to take into account after deciding about the main changes in this PR.

---

Are layouts still necessary in this model? Isn't it the case that packages can put their editable layout in the conanfile? Would be cool if we only needed to specify layouts for those packages that don't have it defined in their recipe.

There is a work in progress trying to embed layouts into the recipe (or infer them), so at some point in time layout files could not be needed anymore.

Anyway, layouts (inferred or not) are still necessary for this PR:

• for packages that are not in the workspace: if these packages are built in a local folder (but not packaged/installed) then layouts are needed to point to include files and libraries. If they were installed/packaged then no layout would be needed.
• for packages that are in the workspace (they are actual CMake targets): we don't need layout files to consume these packages from CMake, from other packages in the workspace; but we do need layout files to consume them from packages that are not included in the workspace.

[Lawrencemm]

I've installed this branch with pip and been testing with my project.

One thing: build_requirements are clashing:

ERROR: Duplicated requirement Catch2/2.5.0@catchorg/stable != Catch2/2.6.1@catchorg/stable

Those versions of Catch are build requirements in two different packages in my workspace.

Not sure if this is intentional, I think it's probably necessary for packages to have requirements that are isolated from others, I know there's some debate about this.

I know you said you mainly want to just figure out if the core concepts of this PR are OK but I think sometimes you don't see conceptual problems without projects working in the wild. So the following are bugs as I've found with my project, I won't be offended if you ignore this but anyway:

The imports of each package are getting copied into the source_folder for each package.

There is a weird problem with PkgConfig, Only with you branch this is failing in one of the editables:

find_package(PkgConfig REQUIRED)
pkg_check_modules(GTKMM REQUIRED gtkmm-3.0)

With the following:

CMake Error at /opt/clion-2019.2.1/bin/cmake/linux/share/cmake-3.14/Modules/FindPkgConfig.cmake:639 (if):
  if given arguments:

    "NOT" "DEFINED" "__pkg_config_checked_GTKMM" "OR" "__pkg_config_checked_GTKMM" "LESS" "OR" "NOT" "GTKMM_FOUND" "OR" "(" "NOT" "gtkmm-3.0" "STREQUAL" "" "AND" "NOT" "REQUIRED;gtkmm-3.0" "STREQUAL" "REQUIRED;gtkmm-3.0" ")" "OR" "(" "gtkmm-3.0" "STREQUAL" "" "AND" "NOT" "REQUIRED;gtkmm-3.0" "STREQUAL" "REQUIRED" ")"

  Unknown arguments specified

If you look at the PkgConfig code for this if statement, you can see the the missing part after "LESS" above is ${PKG_CONFIG_VERSION} and indeed this variable is empty on your branch, but defined as "1" on conan official version 1.18.1. I don't see how there could be an interaction here but there clearly is.

EDIT: another bug, default options seem to be ignored in editables- default_options = {"TBB:shared": True} is getting ignored and is False when I run workspace install, causing me to have to set that option to True on command line.

[jgsogo]

Thanks a lot @Lawrencemm for taking the time to test and experiment with this POC. It's always welcome to know about things that fail and take it for granted that they will be addressed if this PR moves on. Right now IMO it is better to leave it just with the core changes of this alternative implementation.

Some of the problems you mention are quite relevant, default_options should be easy to fix using the lockfiles (options are stored inside), but the imports will need a different approach. About the other errors...

Nevertheless, now it has diverged a lot from develop so maybe some errors no longer apply (and new ones arise). I should rebase this PR... but a lot of changes and conflicts arise, so maybe it is even better to write it from scratch.

[kenfred]

Hi @jgsogo. Can you give an update on the workspaces development? We're languishing back at version 1.12 because our conan workflow broke with the subsequent redesign of workspaces. I would very much like to update to the latest conan version so I can get new features and fixes!

I'd love to get some info on the latest direction to be able to provide feedback. Most important details of my use case have been described in #5762 and related issues. These include:

• The concept that workpaces are not only for temporary co-development of packages but also a way to have multiple packages reside in a single repo and depend on one another. This is critical for using git-submodules and monorepos with multiple packages and consumers.
• The generated output of conan install should be a super build that could invoke the conan install/build/package steps each of the packages independently into the output folder. There are several benefits to this, including automatic support for disparate/heterogeneous build systems. Conan already has a way of adapting all build systems, so why would workspaces limit itself to cmake-only and add_subdirectory?

From what I can see in your description, these don't appear to be things you're planning to support. I'd love to have a discussion on this.

Thanks!

[kenfred]

I thought of other important points that may not be on the radar:

• When you do the super build ([workspaces] Workspaces should be Super Builds #5762), you gain the heterogeneous build ability, but you lose the build dependency tracking you got automatically from add_subdirectory. So the super build makefile (or CMakeLists.txt or whatever) needs to be able to manage the rebuild if anything changes. In cmake, that probably means using add_dependencies to manage the build dependencies and ensure rebuilds happen.
• An important subset of the super build concept is when you have multiple roots that actually need different profiles/toolchains yet still need to depend on each other. For example, if building an embedded device, these are some common use cases:
  • Executable is being build for processor A. Another executable is being built for processor B. They both consume a common library to define a protocol between them. The protocol needs to be built for both platforms and we need to ensure that any change to the protocol forces a rebuild of both executables.
  • A host processor needs to load a separate processor upon boot. The output of the build of the second processor needs to be packaged within the packaging of the host processor. So rebuilding the one should force the rebuild (or at least repackaging) of the other.

In short, we need the super build to manage distinct conan invocations for each root, allowing for individual profiles. This is goes beyond allowing hetergenous toolchains: it allows interdependent conan invocations (hetergeneous settings and platforms)! Yet the super build still needs to track dependencies across the roots so that when one is changed, others will be rebuilt.

[memsharded]

Obsolete, workspace feature removed for 2.0, completely new editables. Lets close it and we will revisit workspaces after 2.0.