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

sim/hostfs: pass flag O_BINARY to host #8997

Merged
merged 1 commit into from Apr 18, 2023

Conversation

anchao
Copy link
Contributor

@anchao anchao commented Apr 11, 2023

Summary

sim/hostfs: pass flag O_BINARY to host

Impact

N/A

Testing

sim/hostfs

@pkarashchenko
Copy link
Contributor

Why "bypass" and not "pass"?

@anchao anchao changed the title sim/hostfs: bypass flag O_BINARY to host sim/hostfs: pass flag O_BINARY to host Apr 11, 2023
@anchao
Copy link
Contributor Author

anchao commented Apr 11, 2023

fix nxstyle mixed case warning:
#8998

@anchao anchao force-pushed the 23041103 branch 2 times, most recently from 15fc948 to 26b511e Compare April 11, 2023 08:25
@anchao anchao requested a review from yamt April 11, 2023 09:30
@anchao anchao force-pushed the 23041103 branch 3 times, most recently from edd79a3 to 83cdc64 Compare April 11, 2023 11:56
@anchao
Copy link
Contributor Author

anchao commented Apr 11, 2023

@pkarashchenko @yamt Could your please review this PR again?

@anchao
Copy link
Contributor Author

anchao commented Apr 12, 2023

CI break on esp32c6-devkit/coremark

====================================================================================
Configuration/Tool: esp32c6-devkit/coremark
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
  Building NuttX...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0

100  475k  100  475k    0     0   715k      0 --:--:-- --:--:-- --:--:--  715k
make: *** [tools/Unix.mk:527: nuttx] Error 2
make: Target 'all' not remade because of errors.
  Normalize esp32c6-devkit/coremark

tools/Unix.mk

526 ifeq ($(CONFIG_INTELHEX_BINARY),y)
527   @echo "CP: nuttx.hex"
528   $(Q) $(OBJCOPY) $(OBJCOPYARGS) -O ihex $(BIN) nuttx.hex
529   $(Q) echo nuttx.hex >> nuttx.manifest
530 endif

command line:

riscv64-unknown-elf-objcopy  -O ihex nuttx nuttx.hex

@acassis @pkarashchenko Any update for the riscv toolchain recently? why would it fail here

@pkarashchenko
Copy link
Contributor

Nothing I'm aware of. Have you tried to rebuild that configuration locally?

@pkarashchenko
Copy link
Contributor

I just tried to rebuild it locally and build was successful

@lucasssvaz
Copy link
Contributor

@anchao @pkarashchenko I'm testing why this config broke. Locally it builds fine as you said.
It appears to be some change that was introduced in the last couple of days and is affecting only the CI. Was there any change to the CI recently ?

@anchao
Copy link
Contributor Author

anchao commented Apr 12, 2023

@pkarashchenko @lucasssvaz It should fail in POSTBUILD, I need some time to align the environment with ci docker, maybe related to esptool:

https://github.com/apache/nuttx/blob/master/tools/esp32c6/Config.mk#L46-L62

@lucasssvaz
Copy link
Contributor

lucasssvaz commented Apr 12, 2023

@pkarashchenko @lucasssvaz It should fail in POSTBUILD, I need some time to align the environment with ci docker, maybe related to esptool:

https://github.com/apache/nuttx/blob/master/tools/esp32c6/Config.mk#L46-L62

I don't know which version of esptool the CI is using but the latest releases had tons of improvements regarding C6 and H2.

EDIT: It appears to be using the latest stable version. I'll check in #9014 if upgrading it has any effect.

EDIT2: Nope, same thing happens. I'll try to build the docker image locally.

@anchao
Copy link
Contributor Author

anchao commented Apr 12, 2023

EDIT2: Nope, same thing happens. I'll try to build the docker image locally.

@lucasssvaz
I can reproduce this problem if force esptool version to 3.3.1:
https://github.com/apache/nuttx/blob/master/tools/ci/cibuild.sh#L289

$ pip install esptool==3.3.1
$ make -j12
CPP:  /home/archer/code/nuttx/n1/nuttx/boards/risc-v/esp32c6/esp32c6-devkit/../common/scripts/esp32c6_rom.ld-> /home/archer/code/nuttx/n1/nuttx/boards/risc-v/esp32c6/esp32c6-devkiCPP:  /home/archer/code/nuttx/n1/nuttx/boards/risc-v/esp32c6/esp32c6-devkit/../common/scripts/flat_memory.ld-> /home/archer/code/nuttx/n1/nuttx/boards/risc-v/esp32c6/esp32c6-devkiCPP:  /home/archer/code/nuttx/n1/nuttx/boards/risc-v/esp32c6/esp32c6-devkit/../common/scripts/legacy_sections.ld-> /home/archer/code/nuttx/n1/nuttx/boards/risc-v/esp32c6/esp32c6-dLD: nuttx
CP: nuttx.hex
MKIMAGE: ESP32-C6 binary
esptool.py --chip esp32c6 elf2image --flash_mode dio --flash_size "4MB" -o nuttx.bin nuttx
usage: esptool [-h] [--chip {auto,esp8266,esp32,esp32s2,esp32s3beta2,esp32s3,esp32c3,esp32c6beta,esp32h2beta1,esp32h2beta2,esp32c2}] [--port PORT] [--baud BAUD]
               [--before {default_reset,usb_reset,no_reset,no_reset_no_sync}] [--after {hard_reset,soft_reset,no_reset,no_reset_stub}] [--no-stub] [--trace]
               [--override-vddsdio [{1.8V,1.9V,OFF}]] [--connect-attempts CONNECT_ATTEMPTS]
               {load_ram,dump_mem,read_mem,write_mem,write_flash,run,image_info,make_image,elf2image,read_mac,chip_id,flash_id,read_flash_status,write_flash_status,read_flash,verify_flash,erase_flash,erase_region,merge_bin,get_security_info,version}
               ...
esptool: error: argument --chip/-c: invalid choice: 'esp32c6' (choose from 'auto', 'esp8266', 'esp32', 'esp32s2', 'esp32s3beta2', 'esp32s3', 'esp32c3', 'esp32c6beta', 'esp32h2beta1', 'esp32h2beta2', 'esp32c2')
make: *** [tools/Unix.mk:527: nuttx] Error 2

I think upgrade the esptool version to 4.5.1 should be able to fix this issue:
#9016

@lucasssvaz
Copy link
Contributor

@anchao Nice catch! I had only checked the Dockerfile, I forgot about the cibuild.sh. Thanks for the help and the patch!

Since the initial default setting in MSVC is text mode ( O_TEXT ):

https://learn.microsoft.com/en-us/cpp/c-runtime-library/text-and-binary-mode-file-i-o?view=msvc-170

In order to unify the translation behavior with unix,
1. set O_BINARY for hostfs as default
2. enable default text mode if the application specifies flag O_TEXT

Signed-off-by: chao an <anchao@xiaomi.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 899be58 into apache:master Apr 18, 2023
24 of 26 checks passed
@jerpelea jerpelea added this to To-Add in Release Notes - 12.2.0 Jun 13, 2023
@jerpelea jerpelea moved this from To-Add to In Progress in Release Notes - 12.2.0 Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants