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

kconfig: Show unsatisfied deps. when assignments don't take #22052

Merged

Conversation

ulfalizer
Copy link
Collaborator

Show which dependencies are unsatisfied when symbols don't get their
assigned value.

For example, assume that FOO below is assigned with CONFIG_FOO=y. Note
that BAR, BAZ, and STR = "hmm" are 'n'.

config FOO
       bool "foo"
       depends on BAR && BAZ && QAZ && STR = "hmm"

config BAR
       def_bool n

config BAZ
       def_bool n

config QAZ
       def_bool y

config STR
       def_string "zmh"

This now prints this warning:

warning: FOO (defined at /home/ulf/z/z/Kconfig:10) was assigned the
value 'y' but got the value 'n'. Check these unsatisfied
dependencies: BAR (=n), BAZ (=n), STR = "hmm" (=n). See ...

Fixes: #21888

@ulfalizer ulfalizer changed the title config: Show unsatisfied deps. when assignments don't take kconfig: Show unsatisfied deps. when assignments don't take Jan 20, 2020
@ulfalizer
Copy link
Collaborator Author

Builds on #22043. Please merge that one first, in case I update it. I'll then rebase this PR.

# on 'sym' (which can also come from e.g. a surrounding 'if'), returns a
# list of all <expr>s with a value less than the value 'sym' was assigned
# ("less" instead of "not equal" just to be general and handle tristates,
# even though Zephyr doesn't use them).
Copy link
Collaborator

Choose a reason for hiding this comment

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

"a value less than", does this means we won't be warned of an unsatisfied symbol when we have

depends on ! A
?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, the expr itself is less than. This should be OK then.

Copy link
Collaborator

@SebastianBoe SebastianBoe Jan 21, 2020

Choose a reason for hiding this comment

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

Does this behave sanely when there are multiple config declarations with different dependencies, thereby creating a "super-config" with each config's dependencies OR'd together?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It will work at least, though it might require some more manual investigation depending on how the dependencies look.

Might get Check these unsatisfied dependencies: FOO || BAR || BAZ (=n), ... for example. The code always just splits on AND here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Forgot to mention: Symbol.direct_dep has the OR of all the dependencies when a symbol is defined in multiple locations.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, the expr itself is less than. This should be OK then.

Yeah, the way it avoids ever warning for CONFIG_FOO=n is because the dependencies can never be less than n. Avoids special-casing it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Might get Check these unsatisfied dependencies: FOO || BAR || BAZ (=n), ... for example.

Nit on this: I think it might be clearer if the expression is parenthetized, so it's clear the (=n) refers to the whole thing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Now puts () around expressions that aren't plain symbols:

Check these unsatisfied dependencies: BAR (=n), BAZ (=n), (BAZ || BAZ) (=n), (STR = "hmm") (=n)

(Added a BAZ || BAZ dependency just to test.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As a side note, it'll always be (=n) in Zephyr, but it could be (=m) in projects that use tristate symbols (used for modules in Linux). Doesn't hurt to show it at least.

@ulfalizer ulfalizer force-pushed the show-unsatisfied-deps branch 2 times, most recently from 57c248b to d6e5405 Compare January 21, 2020 17:53
Show which dependencies are unsatisfied when symbols don't get their
assigned value.

For example, assume that FOO below is assigned with CONFIG_FOO=y. Note
that BAR, BAZ, and STR = "hmm" are 'n'.

    config FOO
           bool "foo"
           depends on BAR && BAZ && QAZ && STR = "hmm"

    config BAR
           def_bool n

    config BAZ
           def_bool n

    config QAZ
           def_bool y

    config STR
           def_string "zmh"

This now prints this warning:

    warning: FOO (defined at /home/ulf/z/z/Kconfig:10) was assigned the
    value 'y' but got the value 'n'. Check these unsatisfied
    dependencies: BAR (=n), BAZ (=n), STR = "hmm" (=n). See ...

Fixes: zephyrproject-rtos#21888

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
@ulfalizer ulfalizer force-pushed the show-unsatisfied-deps branch from d6e5405 to f830cf8 Compare January 22, 2020 18:52
@ulfalizer
Copy link
Collaborator Author

@oyvindronningstad @SebastianBoe
Rebased and ready now. Any complaints?

Copy link
Collaborator

@oyvindronningstad oyvindronningstad left a comment

Choose a reason for hiding this comment

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

Thanks!

@carlescufi carlescufi merged commit ba2f95a into zephyrproject-rtos:master Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Print unmet Kconfig dependency
5 participants