Skip to content

Conversation

@IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Nov 20, 2024

Was e.g.

┌ Warning: Circular dependency detected. Precompilation will be skipped for:
│   Base.PkgId(Base.UUID("eb0c05c4-6780-5852-a67e-5d31d2970b9a"), "ArrayInterfaceTrackerExt")
│   Base.PkgId(Base.UUID("f517fe37-dbe3-4b94-8317-1923a5111588"), "Polyester")
│   Base.PkgId(Base.UUID("0d7ed370-da01-4f52-bd93-41d350b8b718"), "StaticArrayInterface")
│   Base.PkgId(Base.UUID("6a4ca0a5-0e36-4168-a932-d9be78d558f1"), "AcceleratedKernels")
│   Base.PkgId(Base.UUID("244f68ed-b92b-5712-87ae-6c617c41e16a"), "NNlibAMDGPUExt")
│   Base.PkgId(Base.UUID("06b0261c-7a9b-5753-9bdf-fd6840237b4a"), "StaticArrayInterfaceStaticArraysExt")
│   Base.PkgId(Base.UUID("21141c5a-9bdb-4563-92ae-f87d6854732e"), "AMDGPU")
│   Base.PkgId(Base.UUID("9f7883ad-71c0-57eb-9f7f-b5c9e6d3789c"), "Tracker")
└ @ Base.Precompilation precompilation.jl:511

Now
Screenshot 2024-11-21 at 11 20 50 PM

Thanks to @topolarity figuring out proper cycles tracking.

@IanButterworth IanButterworth added the backport 1.11 Change should be backported to release-1.11 label Nov 20, 2024
@topolarity

This comment was marked as resolved.

@IanButterworth

This comment was marked as resolved.

@topolarity
Copy link
Member

We should probably backport to 1.10 too, since we expect that users will be hitting extension cycles a lot more often on that release.

In 1.11 these cycles are supposed to be impossible, except for environment tearing (where you end up with a package graph that doesn't actually ] resolve) and any implementation bugs (some of which were not fixed until.. yesterday 😉)

Still very good to have (esp. on 1.10), just want to be clear about whether this is a "normal" error any more

@topolarity topolarity added the backport 1.10 Change should be backported to the 1.10 release label Nov 21, 2024
Copy link
Member

@topolarity topolarity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great! I think it's much clearer to users what's going on here now.

Thanks for iterating on this @IanButterworth

@topolarity
Copy link
Member

Oh the cycle order does look a bit "backwards" to me

I would have expected the dependency of a package to be above it, not below

@IanButterworth
Copy link
Member Author

I would have expected the dependency of a package to be above it, not below

I think of it the other way. Matching the sentence order for "Foo depends on Bar, which depends on Baz"

Foo
 Bar
  Baz

@topolarity
Copy link
Member

Hmm.. What if we reverse the indentation from greatest to smallest?

  Baz
 Bar
Foo

Does that align our intuitions?

@topolarity topolarity changed the title precompilepkgs: make the circular dep report prettier precompilepkgs: make the circular dep warning clearer and more informative Nov 22, 2024
Copy link
Member

@topolarity topolarity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still find the staircase weird here (and my first instinct is to read it "backwards"), but this is a big improvement and all the functionality looks ship-shape to me

Good to merge, I think!

@IanButterworth
Copy link
Member Author

Yeah, lets see what people make of it and tweak if there's concensus

@topolarity topolarity merged commit 0bedaae into JuliaLang:master Nov 22, 2024
7 checks passed
@topolarity topolarity removed the backport 1.10 Change should be backported to the 1.10 release label Nov 24, 2024
@topolarity
Copy link
Member

topolarity commented Nov 24, 2024

1.10 backport is done, but we should follow-up with 1.11 soon

#56677

topolarity added a commit to topolarity/julia that referenced this pull request Nov 24, 2024
…ative (JuliaLang#56621)

Was e.g.
```
┌ Warning: Circular dependency detected. Precompilation will be skipped for:
│   Base.PkgId(Base.UUID("eb0c05c4-6780-5852-a67e-5d31d2970b9a"), "ArrayInterfaceTrackerExt")
│   Base.PkgId(Base.UUID("f517fe37-dbe3-4b94-8317-1923a5111588"), "Polyester")
│   Base.PkgId(Base.UUID("0d7ed370-da01-4f52-bd93-41d350b8b718"), "StaticArrayInterface")
│   Base.PkgId(Base.UUID("6a4ca0a5-0e36-4168-a932-d9be78d558f1"), "AcceleratedKernels")
│   Base.PkgId(Base.UUID("244f68ed-b92b-5712-87ae-6c617c41e16a"), "NNlibAMDGPUExt")
│   Base.PkgId(Base.UUID("06b0261c-7a9b-5753-9bdf-fd6840237b4a"), "StaticArrayInterfaceStaticArraysExt")
│   Base.PkgId(Base.UUID("21141c5a-9bdb-4563-92ae-f87d6854732e"), "AMDGPU")
│   Base.PkgId(Base.UUID("9f7883ad-71c0-57eb-9f7f-b5c9e6d3789c"), "Tracker")
└ @ Base.Precompilation precompilation.jl:511
```
Now
![Screenshot 2024-11-21 at 11 20
50 PM](https://github.com/user-attachments/assets/6939d834-90c3-4d87-baa9-cf6a4931ca03)

Thanks to @topolarity figuring out proper cycles tracking.

---------

Co-authored-by: Cody Tapscott <[email protected]>
topolarity added a commit to topolarity/julia that referenced this pull request Nov 24, 2024
…ative (JuliaLang#56621)

Was e.g.
```
┌ Warning: Circular dependency detected. Precompilation will be skipped for:
│   Base.PkgId(Base.UUID("eb0c05c4-6780-5852-a67e-5d31d2970b9a"), "ArrayInterfaceTrackerExt")
│   Base.PkgId(Base.UUID("f517fe37-dbe3-4b94-8317-1923a5111588"), "Polyester")
│   Base.PkgId(Base.UUID("0d7ed370-da01-4f52-bd93-41d350b8b718"), "StaticArrayInterface")
│   Base.PkgId(Base.UUID("6a4ca0a5-0e36-4168-a932-d9be78d558f1"), "AcceleratedKernels")
│   Base.PkgId(Base.UUID("244f68ed-b92b-5712-87ae-6c617c41e16a"), "NNlibAMDGPUExt")
│   Base.PkgId(Base.UUID("06b0261c-7a9b-5753-9bdf-fd6840237b4a"), "StaticArrayInterfaceStaticArraysExt")
│   Base.PkgId(Base.UUID("21141c5a-9bdb-4563-92ae-f87d6854732e"), "AMDGPU")
│   Base.PkgId(Base.UUID("9f7883ad-71c0-57eb-9f7f-b5c9e6d3789c"), "Tracker")
└ @ Base.Precompilation precompilation.jl:511
```
Now
![Screenshot 2024-11-21 at 11 20
50 PM](https://github.com/user-attachments/assets/6939d834-90c3-4d87-baa9-cf6a4931ca03)

Thanks to @topolarity figuring out proper cycles tracking.

---------

Co-authored-by: Cody Tapscott <[email protected]>
topolarity added a commit that referenced this pull request Nov 24, 2024
…and more informative (#56621) (#56677)

Was e.g.
```
┌ Warning: Circular dependency detected. Precompilation will be skipped for:
│   Base.PkgId(Base.UUID("eb0c05c4-6780-5852-a67e-5d31d2970b9a"), "ArrayInterfaceTrackerExt")
│   Base.PkgId(Base.UUID("f517fe37-dbe3-4b94-8317-1923a5111588"), "Polyester")
│   Base.PkgId(Base.UUID("0d7ed370-da01-4f52-bd93-41d350b8b718"), "StaticArrayInterface")
│   Base.PkgId(Base.UUID("6a4ca0a5-0e36-4168-a932-d9be78d558f1"), "AcceleratedKernels")
│   Base.PkgId(Base.UUID("244f68ed-b92b-5712-87ae-6c617c41e16a"), "NNlibAMDGPUExt")
│   Base.PkgId(Base.UUID("06b0261c-7a9b-5753-9bdf-fd6840237b4a"), "StaticArrayInterfaceStaticArraysExt")
│   Base.PkgId(Base.UUID("21141c5a-9bdb-4563-92ae-f87d6854732e"), "AMDGPU")
│   Base.PkgId(Base.UUID("9f7883ad-71c0-57eb-9f7f-b5c9e6d3789c"), "Tracker")
└ @ Base.Precompilation precompilation.jl:511
```
Now
![Screenshot 2024-11-21 at 11 20

50 PM](https://github.com/user-attachments/assets/6939d834-90c3-4d87-baa9-cf6a4931ca03)

Co-authored-by: Ian Butterworth <[email protected]>
@KristofferC KristofferC mentioned this pull request Dec 3, 2024
51 tasks
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants