-
Notifications
You must be signed in to change notification settings - Fork 992
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
[bug] Conan 2.X generates invalid transitive dependency info when building project with editable packages #17549
Comments
Hi @Artalus Thanks for your report. I am having a look to your code, the project doesn't seem usable in Conan 2, because it contains package names in uppercase, which are not allowed in Conan 2 and will fail Without trying yet, it seems the main issue is that Actually, it will need more information, that is the first part. So |
It works just fine for me locally right now with 2.11.0, and what few projects in our monorepo currently work with 2.10.2, do not seem to complain about names being UpperCase either. 🤔
That would make sense with the headers provided by our packages themselves - but the problem is with headers coming from |
It works only in
No, it was this way from the very early 2.0 alpha and beta releases, even discussed in the Conan tribe IIRC, and it has been raising the error since the first 2.0 alpha versions like 4 years ago 😅. It solves some of the long standing issues of case-insensitive filesystems and also reduce possible typo-squatting attacks risks.
Ok, yes, I see what you mean know. As the |
The dependency chain is: 3BigProject->2Subproject->1Wrappers->rttr Only So this wouldn't be a bug, but totally expected. With the current dependency definition, the |
Ah, indeed! It did fail that way, and I now vaguely remember seeing similar message when first trying Conan 2 about a year ago.
Fully agree that they shouldn't be added to the compiler input while compiling Is there anything to be done to support this case, aside from throwing As mentioned before, I anticipate there might be better ways at project organization than we currently use. I skimmed over the new tutorial but did not immediately see anything useful for our case. |
It is not an oversight, but mostly avoiding extra costly checks, trying to keep good performance. It means that editable packages can have whatever name, but that is extremely unusual to have packages only in editable mode that are never created. It is not a big deal, the moment you start creating the packages, it will error and you will need to rename all the packages. As everything will be local, nothing really published to Conan remotes or anything like that, it is not that you will break anyone depending on those packages with uppercase, because the packages were never uploaded.
They shouldn't be visible at all, as that promotes bad practices, allows developers to accidentally break encapsulation, etc. If the headers of a dependency are not propagated through the public headers of a package, they are an internal implementation detail that should never be accidentally exposed to consumers. In this case, the
You might have some meta-project that contains the logic, adds the necessary include folders, etc, but that doesn't belong to Conan (at the moment, we are working on the workspace feature, see https://docs.conan.io/2/incubating.html). What Conan knows about is the
Indeed, |
I agree! And all this is being addressed on CMake side with proper But the key difference between Conan-1 and Conan-2 behavior (and the main issue for me) is that Conan-2 now imposes additional limitations above the CMake model. It is not only preventing the accidental ( I just tested the above setup with The meta-project approach sounds awfully vague and "theoretical". I remember once reading about a project where folks were |
But Conan packages do not necessarily use CMake. So whatever PRIVATE/PUBLIC definition a package contains in its
Not sure what you mean. The Conan model is actually more flexible/expressive than the CMake model, as it has larger control over the headers and libs propagation. Only very recently CMake added things like
Because not everything is CMake, there are many users not using CMake or not using CMake only. And what seemed to work in Conan 1 was doing many times the wrong thing. For example it was massively overlinking by default, because it had the wrong propagation model of libraries. I understand that it could be a bit more convenient in Conan 1, but Conan 2 is by far way more correct in the behaviors and SW engineering practices. The tradeoff is that when a package is transitively exposing its dependencies headers, has to define
This is not theoretical. If your What we will try to do in the Workspaces feature is to provide tools or helpers or automation around such concept. There are already many users using Conan 2 with editables, each Conan editable package will have its own Yes, the |
Then how do I utilize this control to solve my current situation?
It never was, is not intended to be right now, and might never be at all. Neither of those 3 CMakeLists are a package to be
Do you imply that we might try to remove all
And it gets kinda harder to give it a try, if one accounts for the past trauma of Conan-2 migration processes. I literally am unsure if I should touch things marked "experimental" in the docs, because I vividly remember how even things not marked as such got butchered with the update! 🙃 Still, understandable. I will check Workspaces with the example from this issue, and it goes well, add it to the list of things to experiment on monorepo. Speaking of experiments... I guess could also try to split the |
I think that is the issue. The If 3bigProject and 2SubProject are not creating Conan packages yet, maybe it would be enough to just limit the It is the challenge of both trying to create independent packages and have a single build-tree which CMake doesn't have a solution, so it is necessary to do something like: if(SINGLE_BUILD_TREE)
add_subdirectory(mydep)
else()
find_project(mydep ...)
endif()
...
# If the imported target name is not the same as the project library name
if(SINGLE_BUILD_TREE)
target_link_libraries(mytarget ... mydep)
else()
target_link_libraries(mytarget ... mydep::mydep)
endif() And this is more or less what users doing this are doing. So if you are not creating Conan packages for those yet, maybe it is possible to simplify it? Not sure what is the
yes, this is the idea I was suggesting above. The So the thing to remove would be the: def layout(self):
self.folders.build = os.getcwd() and let the As an extra hint, if you maintain the editables, and you want the |
Unfortunately this was just an oversimplification for the example needs 😅
TBH I am unsure if we ever need to switch between The main use case to keep the packages independent to some degree, is so that a dev could focus first on development of a
I must admit that I myself do not fully grasp all "why"s behind the current setup, as it was designed long before me. My understanding - If there is another popular workflow to do this without editables - I will gladly research it, as editables have been a bit of a pain in our CI with shared caches (another antipattern, I know)
Not sure I follow. How is this different from the default
, so the |
Describe the bug
(not sure if this is actually a bug, might be my misunderstanding of the dependency model, but things worked just fine in 1.62)
(also this could reeeallly benefit from a better title...)
We have a monorepo with quite a bunch (40+) libraries and executables under the same roof. We use Conan to work with them like this, essentially:
All projects use the same path for generators directory; this is very likely suboptimal, but allows
conan install
to generate aaalll thepackage-Config.cmake
s under one roof, and so theadd_subdirectory(local) + find_package(thirdparty)
approach works seamlessly. In Conan 1, that is.With Conan 2 (tested with 2.10.2 in work repo and 2.11 in example repo) things get worse. With the dependency chain like
BigProject -> Subproject -> Wrappers -> 3rdparty
I now get cases where I can easily buildWrappers
andSubproject
on their own, but building the exact same targets as part of theBigProject
fails, because the include directories for targets in3rdparty
from Conan are not set.How to reproduce it
I made a repro repo here for convenience - but the issue can be reproduced without any CMake, only conanfiles are needed:
Conanfiles...
It is crucial that the packages have a
shared
option, I could not reproduce the issue outside of our monorepo without it. I assume this relates somehow to how Conan deduces package types (shared/static/headeronly/executable) based on the options prescence or something.Here I do
for x in pyreq Wrappers Subproject BigProject ; do conan editable add ./$x ; done
to initialize the "workspace", and then install the dependencies forBigProject
:No need to bother with CMake and sources here, as already after
conan install
I can see the problem. The generatedBigProject/build/Conan/rttr-release-x86_64-data.cmake
file has this:, so the
rttr
target created by Conan forBigProject
is missing the include directories.If instead of Parsing I repeat the same commands for either Subproject or Wrappers, the
-data.cmake
file is generated correctly:We probably do a few things weirdly/incorrectly here; I, for starters, am kinda sus about all the packages using the same
self.folders.build = os.getcwd()
- and therefore overwriting files of each other. I gladly accept any hints on how to improve this setup, but I have been stuck in the groundhog day of "migrate to Conan 2.0" for about a year now, and for now would very much prefer things to Just Work™ without any huge refactorings and breaking compabitility with Conan-1.The text was updated successfully, but these errors were encountered: