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

Unique property definitions from previous compilations for other custom board options persist #1614

Closed
per1234 opened this issue Jan 5, 2022 · 8 comments · Fixed by #1820
Assignees
Labels
conclusion: resolved Issue was resolved topic: code Related to content of the project itself topic: gRPC Related to the gRPC interface type: imperfection Perceived defect in any part of project

Comments

@per1234
Copy link
Contributor

per1234 commented Jan 5, 2022

Describe the bug

The Arduino boards platform system offers a custom board options capability. The platform properties associated with a given option are defined only when that option is selected (e.g., arduino:avr:nano:cpu=atmega328old or Tools > Processor > ATmega328P (Old Bootloader)).

Because the properties defined in boards.txt override those in platform.txt, default property definitions are often made in platform.txt. This allows non-default property definitions to be made only for those board configurations that need it. A prominent example of this practice is the build.extra_flags property (e.g., here).

🐛 If a property was defined in a custom board option of a board previously compiled for, that board option associated definition persists into subsequent compilations with another board option selection that does not define the property.

To Reproduce

Set up

  1. Download this boards.local.txt file: boards.local.txt

    This file defines a some_menu custom board option for the arduino:avr:uno board.

    Click to see file contents
    menu.some_menu=Some Custom Menu
    
    uno.menu.some_menu.bad=Bad Option
    uno.menu.some_menu.bad.build.extra_flags=-bad-build-extraflags
    uno.menu.some_menu.bad.compiler.cpp.extra_flags=-bad-compiler-cpp-extraflags
    
    uno.menu.some_menu.good=Good Option
    uno.menu.some_menu.good.compiler.cpp.extra_flags=-DGOOD_COMPILER_CPP_EXTRA_FLAGS
    
  2. Put the boards.local.txt file in the arduino:avr boards platform installation folder (e.g., ~/.arduino15/packages/arduino/hardware/avr/1.8.4/)

Demo for Arduino IDE 2.x

  1. Quit the IDE if it is running.
  2. Delete the configuration folder:
    • Windows:
      C:\Users\<user name>\AppData\Roaming\arduino-ide\
      
    • Linux:
      ~/.config/arduino-ide/
      
    • macOS:
      ~/Library/Application Support/arduino-ide/
      
    This is necessary due to a bug in the Arduino IDE that causes it to not recognize new board options (variant of https://github.com/arduino/arduino-ide/issues/591)
  3. Start the Arduino IDE.
  4. Select File > Preferences... from the Arduino IDE menus.
  5. Check the box next to "Show verbose output during ☐ compile".
    (This is done only so you can see the flags added to the compilation commands by the custom board option properties.)
  6. Click the OK button.
  7. Select Tools > Board > Arduino AVR Boards > Arduino Uno from the Arduino IDE menus.
  8. Select Tools > Some Custom Menu > Good Option from the Arduino IDE menus.
  9. Select Sketch > Verify/Compile from the Arduino IDE menus.
  10. Wait for compilation to finish.

🐛 Compilation fails unexpectedly:

avr-g++: error: unrecognized command line option '-bad-build-extraflags'

Note that the flag that caused the compilation failure is only defined via the build.extra_flags property of the Tools > Some Custom Menu > Bad Option custom board option. The expected behavior would be that the default empty property definition from platform.txt be used instead.


Note that the compilation commands do contain the expected -DGOOD_COMPILER_CPP_EXTRA_FLAGS flag which was defined via the compiler.cpp.extra_flags property of the Tools > Some Custom Menu > Good Option custom board option, which proves that option was used:

Detecting libraries used...
"C:\\Users\\per\\AppData\\Local\\Arduino15\\packages\\arduino\\tools\\avr-gcc\\7.3.0-atmel3.6.1-arduino7/bin/avr-g++" -c -g -Os -w -std=gnu++11 -fpermissive -fno-exceptions -ffunction-sections -fdata-sections -fno-threadsafe-statics -Wno-error=narrowing -flto -w -x c++ -E -CC -mmcu=atmega328p -DF_CPU=16000000L -DARDUINO=10607 -DARDUINO_AVR_UNO -DARDUINO_ARCH_AVR -DGOOD_COMPILER_CPP_EXTRA_FLAGS -bad-build-extraflags "-IC:\\Users\\per\\AppData\\Local\\Arduino15\\packages\\arduino\\hardware\\avr\\1.8.4\\cores\\arduino" "-IC:\\Users\\per\\AppData\\Local\\Arduino15\\packages\\arduino\\hardware\\avr\\1.8.4\\variants\\standard" "C:\\Users\\per\\AppData\\Local\\Temp\\arduino-sketch-762B53F80F856C25998A7E9799EC2C98\\sketch\\sketch_jan4a.ino.cpp" -o nul

This behavior is extra confusing since the erroneous property definition could only have been produced by a compilation for the Tools > Some Custom Menu > Bad Option custom board option, but no compilation was initiated for that option in this demo. The explanation is that the Arduino IDE automatically compiled for that option as soon as you selected the Arduino Uno board in order to generate the language server data.

Demo for Arduino CLI

  1. Use the gRPC interface to Create and Init an "Arduino Core" instance.
  2. Use that instance to compile any valid sketch (e.g., arduino-cli sketch new) for arduino:avr:uno:some_menu=bad
    🙂 The compilation fails as expected due to the invalid compiler flags specified by this board option configuration:
    avr-g++: error: unrecognized command line option '-bad-compiler-cpp-extraflags'
    avr-g++: error: unrecognized command line option '-bad-build-extraflags'
    
  3. Use that instance to compile any valid sketch for arduino:avr:uno:some_menu=good

🐛 Compilation fails unexpectedly:

avr-g++: error: unrecognized command line option '-bad-build-extraflags'

Expected behavior

Property definitions from previous compilations should not be persistent.

Desktop

  • OS: Windows 10
  • Version: git-snapshot Commit: 60c1c98 Date: 2022-01-05T01:37:22Z

Additional context

Originally reported at https://forum.arduino.cc/t/stm32f-board-will-not-select/941102

Although I demonstrated the issue via a minimal contrived platform configuration, the report was from a user of the STMicroelectronics:stm32 ("STM32Duino") boards platform, which provides a real world occurrence of this bug. You can reproduce it using that platform via the following instructions:

Click to see instructions
  1. Start the Arduino IDE.
  2. Select File > Preferences from the Arduino IDE menus.
  3. Add the following URL:
    https://github.com/stm32duino/BoardManagerFiles/raw/main/package_stmicroelectronics_index.json
    
  4. Click the OK button.
  5. Select File > Quit from the Arduino IDE menus.
    (this is necessary due to the unrelated bug https://github.com/arduino/arduino-ide/issues/637)
  6. Start the Arduino IDE.
  7. Select File > New from the Arduino IDE menus.
  8. Use Boards Manager to install the "STM32 MCU based boards" platform.
  9. Select Tools > Board > STM32 MCU based boards > Generic STM32F4 series" from the Arduino IDE menus.
  10. Select Tools > Board part number > Generic F401CCYx" from the Arduino IDE menus.
    (This particular board configuration is selected arbitrarily for the demo. Other board configurations in this platform are also affected by the bug.)
  11. Select Sketch > Verify/Compile from the Arduino IDE menus.
    🐛 Compilation fails unexpectedly:
    <command-line>: fatal error: variant_BLACK_F407VX.h: No such file or directory
    

This error is caused by a property definition from another option of the "Board part number" menu than the "Generic F401CCYx" option that was selected:
https://github.com/stm32duino/Arduino_Core_STM32/blob/2.2.0/boards.txt#L2117

The "Generic F401CCYx" option that was selected does not define the property:
https://github.com/stm32duino/Arduino_Core_STM32/blob/2.2.0/boards.txt#L2270-L2276
so the default definition should have been used:
https://github.com/stm32duino/Arduino_Core_STM32/blob/2.2.0/platform.txt#L56


Another sighting of this bug here:
https://forum.arduino.cc/t/problems-with-megatinycore-in-ide-2/959081

@ObviousInRetrospect
Copy link

So I just spent tens of hours debugging a clock fuse issue caused by the defect described here: https://forum.arduino.cc/t/problems-with-megatinycore-in-ide-2/959081 which links to this bug.

This probably should be a far higher severity. It causes incorrect fuse settings which in some cases can brick the hardware.

This does not occur on 1.x

@per1234 per1234 assigned cmaglie and unassigned silvanocerza Jul 30, 2022
@ObviousInRetrospect
Copy link

For what its worth, the maintainer of megaTinyCore (https://github.com/SpenceKonde/megaTinyCore) just added:

WARNING: NOT COMPATIBLE WITH 2.0.x version of the IDE due to critical regressions.

These bugs in the IDE prevent board settings from being correctly recognized. Please direct your complaints to the Arduino team. We do not intend to make any effort to support working around the errors of the arduino team in beta software. Working around it in released versions is hard enough. If and when the third party hardware is ammededed with a clear description of the intended behavior, I will fix it. I am not going to fix or allow fixes for bugs thare aren't even acknowledged by the Arduino team as such and may or may not be considerered intended.

to the top of his documentation.

@SpenceKonde
Copy link

I consider this issue to be show stopper as far as ATTinyCore, DxCore and megaTinyCore are concerned. A workaound wouldr require hundreds or thousands of lines added to boardst .txt.

Those three cores do not and never will support any IDE version with such a severe bug with such a challenging workaround.

@matthijskooijman
Copy link
Collaborator

@per1234, you reported this bug and documented reproduction using the IDE 2.0 and the grpc interface only. Does that mean the issue does not occur when using the commandline arduino-cli verify? I guess that makes sense, since then you have a clean arduino-cli start and it would be hard for anything from a previous build to leak into a subsequent build, but for the grpc interface (which is also used by IDE 2.0 I think), that could be a matter of not clearing any previous options (maybe the implementation just applies any custom options when selected, instead of restarting with a clean slate?)

@per1234
Copy link
Contributor Author

per1234 commented Aug 1, 2022

Does that mean the issue does not occur when using the commandline arduino-cli verify?

Correct. It is specific to the "daemon" mode because there you have a persistent instance which can be used to do multiple compilations for various boards.

which is also used by IDE 2.0 I think

That is correct.

@SpenceKonde
Copy link

SpenceKonde commented Aug 1, 2022 via email

cmaglie added a commit to cmaglie/arduino-cli that referenced this issue Aug 1, 2022
cmaglie added a commit to cmaglie/arduino-cli that referenced this issue Aug 1, 2022
cmaglie added a commit to cmaglie/arduino-cli that referenced this issue Aug 1, 2022
@cmaglie cmaglie closed this as completed in 2dd8976 Aug 2, 2022
cmaglie added a commit to cmaglie/arduino-cli that referenced this issue Aug 2, 2022
@ObviousInRetrospect
Copy link

Thanks for fixing this!

I went ahead and reproduced the issue in RC9.1 and then confirmed that dropping the 08/03 nightly cli build into /Applications/Arduino IDE.app/Contents//Resources/app/node_modules/arduino-ide-extension/build fixes the issue.

Since I assume the nightly builds on the IDE side pick up the configured cli release rather than the nightly it might be worth pushing out a cli release that includes it soonish so the nightly builds on the ide side can get the fix.

@cmaglie
Copy link
Member

cmaglie commented Aug 3, 2022

Thanks for the feedback @ObviousInRetrospect!

Since I assume the nightly builds on the IDE side pick up the configured cli release rather than the nightly it might be worth pushing out a cli release that includes it soonish so the nightly builds on the ide side can get the fix.

that's correct, we pin a specific commit of the Arduino CLI to avoid breaking changes in the gRPC API, but we can manually move the pinned version to the current master: arduino/arduino-ide#1280 as soon as the CI passes you will have an Arduino IDE 2.x nightly with the fix.

@per1234 per1234 added the conclusion: resolved Issue was resolved label Aug 3, 2022
bearer-pipeline-test pushed a commit to BearerPipelineTest/arduino-cli that referenced this issue Aug 10, 2022
…ests in go (WIP) (arduino#1806)

* testsuite: Added helper functions to handle test envs

* testsuite: Added helper functions to run arduino-cli

* testsuite: Added colored output to arduino-cli Run helper

* testsuite: Pass cli config through env variables

* testsuite: added env commands to download and extract files

* testsuite: added commands to start cli daemon and run some gRPC calls

* testsuite: moved test harness inside 'internal' package

* testsuite: added first daemon test for gRPC board watch

* testsuite: added http server helper

* testsuite: added JSON helpers

* testsuite: Added possibility to use shared download staging folder

* testsuite: Converted a core_test.py test (WIP)

* REMOVEME: Deactivate daemon integration test for now

* testsuite: force colored output

* testsuite: moved all generic subroutines into their own library

* testsuite: moved daemon test in his own dir

* fixed typo

* testsuite: added some helpers to improve daemon testing

* testsuite: added test for arduino#1614

* Removed converted test

* testsuite: perform build before test

Otherwise people running the task locally will have a bad time when the test
either fails due to the missing executable, or far worse when they get
incorrect test results because they don't realize it is using whichever random
executable happened to be sitting around from the last time they did a build.

* Added missing comment

* Renamed test file

* Skip failing tests

* Updated licensed cache

* re-enable test for fixed bug

* testsuite: disable postinstall

Otherwise Windows CI will get stuck.

* Removed useless startDaemon

* Renamed inst -> grpcInst

* Close daemon gRPC connection on test cleanup

* Added comment

* Apply suggestions from code review

Co-authored-by: per1234 <[email protected]>

* Update internal/integrationtest/arduino-cli.go

Co-authored-by: per1234 <[email protected]>

Co-authored-by: per1234 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: resolved Issue was resolved topic: code Related to content of the project itself topic: gRPC Related to the gRPC interface type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants