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

[BUG] cmake savedefconfig failure after https://github.com/apache/nuttx/pull/13815 #14281

Closed
1 task done
xuxin930 opened this issue Oct 15, 2024 · 12 comments · Fixed by #14401
Closed
1 task done

[BUG] cmake savedefconfig failure after https://github.com/apache/nuttx/pull/13815 #14281

xuxin930 opened this issue Oct 15, 2024 · 12 comments · Fixed by #14401
Labels
Arch: all Issues that apply to all architectures Area: Build system Area: Kconfig Kconfig issues

Comments

@xuxin930
Copy link
Contributor

Description / Steps to reproduce the issue

Important

cmake savedefconfig failure after #13815

Steps to reproduce the problem

1. config with CMake

cmake -B build -DBOARD_CONFIG=qemu-armv7a:nsh

2. change any config with menuconfig

cmake --build build -t menuconfig
# For example  Disable CONFIG_EXAMPLES_HELLO

3. savedefconfig use cmake

cmake --build build -t savedefconfig

Issue: Neither the BINARY directory nor the defconfig in the config directory has been modified.
Expect: Both are modified to the latest

This issue did not exist before this PR: #13815

On which OS does this issue occur?

[OS: Linux]

What is the version of your OS?

Ubuntu 20.04

NuttX Version

12.5.1

Issue Architecture

[Arch: all]

Issue Area

[Area: Build System], [Area: Kconfig]

Verification

  • I have verified before submitting the report.
@github-actions github-actions bot added Arch: all Issues that apply to all architectures Area: Build system Area: Kconfig Kconfig issues labels Oct 15, 2024
@xuxin930
Copy link
Contributor Author

hi @simbit18 can you take a look at this issue?

@simbit18
Copy link
Contributor

hi @xuxin930 sure !!!

@simbit18
Copy link
Contributor

simbit18 commented Oct 18, 2024

@xuxin930 Clarification

3. savedefconfig use cmake
cmake --build build -t savedefconfig
Issue: Neither the BINARY directory nor the defconfig in the config directory has been modified.
Expect: Both are modified to the latest

Why should savedefconfig replace the defconfig file in the boards/arm/qemu-armv7a/configs/nsh path?
Its task (as in compiling with make) should only be to recreate the defconfig file in the cmake buid path.
https://github.com/ulfalizer/Kconfiglib/blob/master/savedefconfig.py

Perhaps you should have a target with a different name like replacedefconfig.

It is also better to rename the current savedefconfig with refreshsilent

simbit18 added a commit to simbit18/nuttx that referenced this issue Oct 18, 2024
Renamed savedefconfig -> refreshsilent name more consistent with the refresh stage for cmake on github.

Added replacedefconfig to fix
apache#14281
@xuxin930
Copy link
Contributor Author

xuxin930 commented Oct 18, 2024

yes @simbit18
Thank you for your reminder. You are right.
We just need to ensure that the defconfig saved in BINARY_DIR is what we expect.

@simbit18
Copy link
Contributor

simbit18 commented Oct 18, 2024

@xuxin930 I also created replacedefconfig simbit18@2061d3a
Do I need it or remove it?

@xuxin930
Copy link
Contributor Author

@xuxin930 I also created replacedefconfig simbit18@2061d3a Do I need it or remove it?

I think rename is unnecessary, as it has different semantics from Kconfig.
How about keeping the name savedefconfig?

@simbit18
Copy link
Contributor

@xuxin930 OK then I rename current savedefconfig to refreshsilent
and my replacedefconfig to savedefconfig confirm ?

@xuxin930
Copy link
Contributor Author

This seems to solve the problem I reported.

because after modifying the config through menuconfig on the Makefile base,
then executing savedefconfig, we can get the modified defconfig

@simbit18
Copy link
Contributor

simbit18 commented Oct 18, 2024

OK I add PR.
#14401

simbit18 added a commit to simbit18/nuttx that referenced this issue Oct 18, 2024
Renamed savedefconfig -> refreshsilent name more consistent with the refresh stage for cmake on github.

Added new savedefconfig to fix
apache#14281
simbit18 added a commit to simbit18/nuttx that referenced this issue Oct 18, 2024
Renamed savedefconfig -> refreshsilent name more consistent with the refresh stage for cmake on github.

Added new savedefconfig to fix
apache#14281
@raiden00pl
Copy link
Member

Why should savedefconfig replace the defconfig file in the boards/arm/qemu-armv7a/configs/nsh path?
Its task (as in compiling with make) should only be to recreate the defconfig file in the cmake buid path.
https://github.com/ulfalizer/Kconfiglib/blob/master/savedefconfig.py

@simbit18 look at this PR discussion: #11416
FYI @anchao

@xuxin930
Copy link
Contributor Author

Why should savedefconfig replace the defconfig file in the boards/arm/qemu-armv7a/configs/nsh path?
Its task (as in compiling with make) should only be to recreate the defconfig file in the cmake buid path.
https://github.com/ulfalizer/Kconfiglib/blob/master/savedefconfig.py

@simbit18 look at this PR discussion: #11416 FYI @anchao

thanks @raiden00pl
our original intention is to be consistent with the behavior of Makefile.
That is, only save the defconfig of BINARY_DIR, not the original defconfig of the board.

@simbit18
Copy link
Contributor

simbit18 commented Oct 18, 2024

@raiden00pl
refreshsilent (PR #14401) behaves with the same logic as the tools/refresh.sh script used for the refresh stage (make build) on GitHub.

See https://github.com/apache/nuttx/blob/master/tools/refresh.sh

example nucleo-l152re/nsh

./tools/refresh.sh --silent nucleo-l152re/nsh

  • rm -f SAVEconfig
  • mv .config SAVEconfig
  • cp -a boards/arm/stm32/nucleo-l152re/configs/nsh/defconfig .config
  • make olddefconfig
  • make savedefconfig
  • diff boards/arm/stm32/nucleo-l152re/configs/nsh/defconfig defconfig

has been merge with this PR #13815

without generating this error on github
#13815 (comment)

@xiaoxiang781216 xiaoxiang781216 linked a pull request Oct 18, 2024 that will close this issue
xiaoxiang781216 pushed a commit that referenced this issue Oct 18, 2024
Renamed savedefconfig -> refreshsilent name more consistent with the refresh stage for cmake on github.

Added new savedefconfig to fix
#14281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: all Issues that apply to all architectures Area: Build system Area: Kconfig Kconfig issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants