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

Circular dependencies if adding libunwind as dependency #8

Closed
wants to merge 1 commit into from

Conversation

DeliaPavel
Copy link
Contributor

libunwind already specifies compiler-rt as a dependency in its Config.uk. If we would add libunwind as a dependency in compiler-rt's Config.uk we would receive an error because there would be a circular dependency.

Signed-off-by: Delia-Maria Pavel [email protected]

@DeliaPavel
Copy link
Contributor Author

regarding #4

Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

Looks good.
Reviewed-by: Stefan Jumarea [email protected]

@DeliaPavel DeliaPavel force-pushed the staging branch 2 times, most recently from 2bcc56e to ce4bd23 Compare September 10, 2022 13:58
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

See my comment. I tested it and with the Config.uk contents below, it works:

menuconfig LIBCOMPILER_RT
    bool "compiler-rt - runtime support"
    select LIBPOSIX_SYSINFO
    select LIBUKMMAP
    depends on LIBUNWIND
    default n

if LIBCOMPILER_RT
    config LIBCOMPILER_RT_ATOMIC
    bool "Implementation of an atomics library"
    select CXX_THREADS
    default N
endif

I user app-helloworld-cpp to test.

Config.uk Outdated Show resolved Hide resolved
@DeliaPavel DeliaPavel force-pushed the staging branch 2 times, most recently from 1bd5bf3 to e613b45 Compare September 10, 2022 15:35
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

See comment below.

@razvand razvand linked an issue Sep 10, 2022 that may be closed by this pull request
@razvand razvand added enhancement New feature or request kind/hackathon Hackathon challenge labels Sep 10, 2022
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

@DeliaPavel , together with @StefanJum we did a thorough analysis of the dependencies. This should be fixed. lib-compiler-rt depends on lib-libunwind. select LIBUNWIND should be part of this. And select COMPILER_RT needs to be removed from the Config.uk file for lib-libunwind.

@StefanJum
Copy link
Member

@DeliaPavel , together with @StefanJum we did a thorough analysis of the dependencies. This should be fixed. lib-compiler-rt depends on lib-libunwind. select LIBUNWIND should be part of this. And select COMPILER_RT needs to be removed from the Config.uk file for lib-libunwind.

The select COMPILER_RT line is removed from lib-libunwind (see this pr).
So @DeliaPavel, whenever you have some time modify this pr to select libunwind.

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Thanks, @DeliaPavel .

Reviewed-by: Razvan Deaconescu [email protected]
Approved-by: Razvan Deaconescu [email protected]

Copy link
Member

@mogasergiu mogasergiu left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Sergiu Moga [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/merged enhancement New feature or request kind/hackathon Hackathon challenge
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Does not automatically select dependencies
5 participants