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: Make 'source' non-globbing and use 'gsource' #7402

Merged
merged 1 commit into from
May 8, 2018

Conversation

ulfalizer
Copy link
Collaborator

@ulfalizer ulfalizer commented May 8, 2018

Until now, Zephyr has used a patched Kconfiglib that turns 'source' into
a globbing source (by replacing 'source' with 'gsource' at the token
level). There's two problems with this:

  • The patch needs to be maintained separately

  • Misspelled filenames are silently ignored, as they look like glob
    patterns that don't match anything

Fix it as follows:

  1. Replace all 'source' statements that use wildcards with 'gsource'

  2. Remove the custom Kconfiglib patch so that 'source' no longer globs

The sed pattern /source.*[*?]/s/source/gsource/ was run over all
Kconfig* files to do the replacement.

source's that use environment variables that might contain glob patterns
were manually changed to gsource.

Building the docs in doc/ is a good test, as doc/Makefile deliberately
sets the environment variables to glob up as many Kconfig files as
possible.

Signed-off-by: Ulf Magnusson ulfalizer@gmail.com

Until now, Zephyr has used a patched Kconfiglib that turns 'source' into
a globbing source (by replacing 'source' with 'gsource' at the token
level). There's two problems with this:

  - The patch needs to be maintained separately

  - Misspelled filenames are silently ignored, as they look like glob
    patterns that don't match anything

Fix it as follows:

  1. Replace all 'source' statements that use wildcards with 'gsource'

  2. Remove the custom Kconfiglib patch so that 'source' no longer globs

The sed pattern '/source.*[*?]/s/source/gsource/' was run over all
Kconfig* files to do the replacement.

source's that use environment variables that might contain glob patterns
were manually changed to gsource.

Building the docs in doc/ is a good test, as doc/Makefile deliberately
sets the environment variables to glob up as many Kconfig files as
possible.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
@ulfalizer
Copy link
Collaborator Author

CC @carlescufi @SebastianBoe @nashif

@codecov-io
Copy link

Codecov Report

Merging #7402 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7402   +/-   ##
=======================================
  Coverage   55.26%   55.26%           
=======================================
  Files         467      467           
  Lines       51641    51641           
  Branches     9886     9886           
=======================================
  Hits        28541    28541           
  Misses      19190    19190           
  Partials     3910     3910

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5dc2ab4...f53d961. Read the comment docs.

Copy link
Collaborator

@SebastianBoe SebastianBoe left a comment

Choose a reason for hiding this comment

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

This change is very welcome because it re-aligns the definition of 'source' with the kernel and it allows us to now express

# Include this Kconfig file

Whereas before we could only express

# Include this Kconfig file if it exists

in the Kconfig language.

Copy link
Member

@carlescufi carlescufi 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, finally got there.

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.

4 participants