conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.27k stars 981 forks source link

[question] Build order inconsistency? #6510

Closed radonish closed 8 months ago

radonish commented 4 years ago

I'm seeing an inconsistency with conan graph build-order. Sometimes the package I'm obtaining the build-order for is itself included in the build order (as the last item) which is very convenient/preferred for my Jenkins code, but sometimes the package is not included.

[libA, libB, libC, appX] vs. [libA, libB, libC]

In fact, the reason I discovered #6482 while trying 1.22.0 for the first time is because the conan upload failure was happening for the final item in the build-order, "appX". Tonight, I'm seeing cases where appX is left out of the build-order and a new graph is not detecting that appX needs to be built. If I make an appY that depends on a component of appX - the build-order for appY does detect the need to rebuild appX so it feels like something is wrong.

I don't recall ever seeing this inconsistency on 1.21.x

Was there an intentional change related to build-order sometimes excluding the package you are requesting the build-order for?

Thanks

jgsogo commented 4 years ago

Hi! We are modifying the graphlocks to allow dynamic nodes on them: if you change the version of a package it can contain new requirements, deprecate others,... if there are build_requires, they are only in the graph is the consumer is to be built... with this changes we are facing many challenges and some issues.

But, at any moment, the behavior (even if buggy) should be deterministic. So, when you get a different output for the build-order, is it the same Jenkins job (same graphlock file, same Conan profile, same arguments? Can you provide more information about it so we can reproduce the issue?

Thanks a lot!

jgsogo commented 4 years ago

Hi, @radonish , any feedback on this? is it still happening?

radonish commented 4 years ago

Hello @jgsogo - I've updated to version 1.22.3 and will hopefully be able to test this out next week. I'll update at that point.

Thanks!

radonish commented 4 years ago

Hello @jgsogo, to follow up: I'm consistently seeing the package whose build order I am asking for not included in the build order list at this point. I'm now using version 1.23.0. I suspect it was an older version I was seeing the package included (appX in my example in the original post) all the time.

Let me say, having appX included in the build order list would be extremely helpful. Our monolithic repo looks something like this: repo/libA repo/libB repo/libC repo/appX repo/appY

A git commit triggers a CI build that goes something like this:

  1. Get build order for appX
  2. Spawn parallel jobs for appX build order libs
  3. Build appX
  4. Get build order for appY
  5. Spawn parallel jobs for appY build order libs
  6. Build appY

Steps 3. and 6. - the building of the apps - is a waste of time currently in the cases where that specific app did not need rebuilding. We are using package_revision_mode. This issue goes away if Conan goes back to how it used to work where the package whose

Should I create a separate feature addition post?

Thanks!

jgsogo commented 4 years ago

Hi! We really want lockfiles to be a stable feature and we are working hard to achieve that objective. We know it is needed to implement a working CI and it is one of the things we are currently focused on, as you can read we had prepared a training for the ConanDays (https://conandays.conan.io/). BTW, if you are interested in that training you can write to us and we'll manage, we are organizing these trainings online.

I'm running a simple example with a graph like appX -> libA -> libB and I agree with you it is not the best UX, nevertheless I get a consistent result with all the libraries/apps needed to build if I add the --build missing argument:

conan graph build-order conan.lock --build missing

A note about the package_revision_mode, it is encoding in the package_id also the package revision of the requirements so you will need to rebuild every consumer any time a library changes, even if you just rebuild the same sources (C++ builds are not reproducible). I think we have a couple of cool blogposts about package_id and build reproducibility.


Let me know if using --build missing helps you with your CI. We will keep working on lockfiles and we will keep working to provide a Conan recommendation for a C++ CI/CD. Stay tuned!

radonish commented 4 years ago

Hello, @jgsogo - I am running using conan graph as you describe: cd appX/ conan graph lock . --profile profile-x86_64-rhel --lockfile conan.lock --build=missing conan graph build-order conan.lock --json=build_order.json --build missing

Now I never get appX within the build order even though appX clearly needs to build because lots of requirements required building - you are saying you do in some cases?

jgsogo commented 4 years ago

I'm generating the graph lock using the reference:

⇒  conan graph lock appX/version@  

Requirements
    appX/version from local cache - Cache
    libA/version from local cache - Cache
    libB/version from local cache - Cache
Packages
    appX/version:Package_ID_unknown - Unknown
    libA/version:6bffca4d0824e8aaa46d94a345337947746fb228 - Missing
    libB/version:103f6067a947f366ef91fc1b7da351c588d1827f - Cache

and then the build-order:

⇒  conan graph build-order conan.lock --build missing
Using lockfile: '/Users/jgsogo/dev/conan/tmp/6510/conan.lock'
Requirements
    appX/version from local cache - Cache
    libA/version from local cache - Cache
    libB/version from local cache - Cache
Packages
    appX/version:Package_ID_unknown - Unknown
    libA/version:6bffca4d0824e8aaa46d94a345337947746fb228 - Build
    libB/version:103f6067a947f366ef91fc1b7da351c588d1827f - Cache

[[('2', 'libA/version#05c1d887c0c427a31c6a1df9144ce8bf:6bffca4d0824e8aaa46d94a345337947746fb228')], [('1', 'appX/version#c8e66da9817fba599b6de2404f1dd66a:Package_ID_unknown')]]

Indeed, if I run it using a path, I get the same as you:

⇒  conan graph lock . --build missing

Requirements
    libA/version from local cache - Cache
    libB/version from local cache - Cache
Packages
    libA/version:6bffca4d0824e8aaa46d94a345337947746fb228 - Build
    libB/version:103f6067a947f366ef91fc1b7da351c588d1827f - Cache
⇒  conan graph build-order conan.lock --build missing

Using lockfile: '/Users/jgsogo/dev/conan/tmp/6510/appX/conan.lock'
Requirements
    libA/version from local cache - Cache
    libB/version from local cache - Cache
Packages
    libA/version:6bffca4d0824e8aaa46d94a345337947746fb228 - Build
    libB/version:103f6067a947f366ef91fc1b7da351c588d1827f - Cache

[[('1', 'libA/version#05c1d887c0c427a31c6a1df9144ce8bf:6bffca4d0824e8aaa46d94a345337947746fb228')]]

But I get the same output for several Conan versions (the ID of the nodes changes, but the listed nodes are the same). Can you share something else so I can reproduce your previous output?


Conan behaves a bit differently if you use an installed reference or a path to a conanfile. It is the same when you run conan install, if using a reference Conan will install that reference and its dependencies, if you use a path Conan will just install the dependencies. This behavior in the conan graph build-order I think is consistent.

In your CI, it should be easy to change your commands to: conan export ... + conan graph lock <reference> + conan graph build-order ..., does it make sense?

Even though we cannot consider it a regression, your feedback will very useful, and maybe we realize that it is better to change the output to facilitate a working CI. The more you share with us about how you are configuring your pipeline, the better for us to understand your needs.

Thanks

radonish commented 4 years ago

@jgsogo - the behavior difference between using a reference and a path is very interesting. Thank you for pointing that out. I am now thinking that when I saw the different behavior I may have been using the full reference as opposed to a relative path.

For now I have changed our Jenkins/CI code to use the full reference but longer-term it would really benefit us to have the same behavior (having appX included in the build order) when using a relative path as well.

Thanks for your help!

memsharded commented 8 months ago

Closing this as stale, and most likely outdated with Conan 2