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

arm, arm64, xtensa, libxx: Change sed -r to sed -E to support macOS #41

Closed
wants to merge 1 commit into from

Conversation

lupyuen
Copy link

@lupyuen lupyuen commented Oct 10, 2023

Summary

When we build NuttX on macOS, it shows many sed messages (and the build still completes successfully):

$ tools/configure.sh pinephone:nsh
$ make
sed: illegal option -- r

(See the Complete Log)

This is due to the Makefiles executing sed -r which is not a valid option on macOS. (As explained here)

This PR proposes to change sed -r to sed -E because:

  1. sed -E on macOS is equivalent to sed -r on Linux (See this)

  2. sed -E and sed -r are aliases according to the GNU sed Manual

  3. sed -E is already used in nuttx_add_romfs.cmake, nuttx_add_symtab.cmake and process_config.sh

Impact

On macOS: NuttX now builds without showing any sed messages.

On other platforms: There should be no impact, since sed -E and sed -r are functionally identical.

Testing

sed is used by the Makefiles to extract the GCC Major Version and the Clang Major / Minor Version. We tested the sed commands on Linux and macOS to verify that they work correctly:

## sed 4.9 on Linux 6.3.9 (Manjaro Arm64)
$ aarch64-none-elf-gcc --version
aarch64-none-elf-gcc (Arm GNU Toolchain 12.2.Rel1 (Build arm-12.24)) 12.2.1 20221205

$ aarch64-none-elf-gcc --version | grep gcc | sed -E "s/.* ([0-9]+\.[0-9]+).*/\1/" | cut -d'.' -f1
12

$ clang --version
clang version 15.0.7

$ clang --version | grep "clang version" | sed -E "s/.* ([0-9]+\.[0-9]+).*/\1/"
15.0

## sed on macOS 10.15.7
$ aarch64-none-elf-gcc --version
aarch64-none-elf-gcc (Arm GNU Toolchain 11.3.Rel1) 11.3.1 20220712

$ aarch64-none-elf-gcc --version | grep gcc | sed -E "s/.* ([0-9]+\.[0-9]+).*/\1/" | cut -d'.' -f1
11

$ clang --version
clang version 3.9.1 (tags/RELEASE_391/final)

$ clang --version | grep "clang version" | sed -E "s/.* ([0-9]+\.[0-9]+).*/\1/"
3.9

On macOS: NuttX now builds without showing any sed messages.

@lupyuen lupyuen marked this pull request as draft October 10, 2023 04:00
@lupyuen lupyuen changed the title Change sed -r to sed -E to support macOS arm, arm64, xtensa, libxx: Change sed -r to sed -E to support macOS Oct 10, 2023
When we build NuttX on macOS, it shows many `sed` messages (and the build still completes successfully):

```text
$ tools/configure.sh pinephone:nsh
$ make
sed: illegal option -- r
```

This is due to the Makefiles executing `sed -r` which is not a valid option on macOS.

This PR proposes to change `sed -r` to `sed -E` because:

- `sed -E` on macOS is equivalent to `sed -r` on Linux

- `sed -E` and `sed -r` are aliases according to the GNU `sed` Manual

- `sed -E` is already used in nuttx_add_romfs.cmake, nuttx_add_symtab.cmake and process_config.sh
@lupyuen lupyuen closed this Oct 10, 2023
lupyuen pushed a commit that referenced this pull request Oct 15, 2024
set CONFIG_PRIORITY_INHERITANCE=y
set CONFIG_SEM_PREALLOCHOLDERS=0 or CONFIG_SEM_PREALLOCHOLDERS=8

    #24 0x4dcab71 in __assert assert/lib_assert.c:37
    #25 0x4d6b0e9 in nxsem_destroyholder semaphore/sem_holder.c:602
    #26 0x4d80cf7 in nxsem_destroy semaphore/sem_destroy.c:80
    #27 0x4d80db9 in sem_destroy semaphore/sem_destroy.c:120
    #28 0x4dcb077 in nxmutex_destroy misc/lib_mutex.c:122
    #29 0x4dc6611 in pipecommon_freedev pipes/pipe_common.c:117
    #30 0x4dc7fdc in pipecommon_close pipes/pipe_common.c:397
    #31 0x4ed4f6d in file_close vfs/fs_close.c:78
    #32 0x6a91133 in local_free local/local_conn.c:184
    #33 0x6a92a9c in local_release local/local_release.c:129
    #34 0x6a91d1a in local_subref local/local_conn.c:271
    #35 0x6a75767 in local_close local/local_sockif.c:797
    #36 0x4e978f6 in psock_close socket/net_close.c:102
    #37 0x4eed1b9 in sock_file_close socket/socket.c:115
    #38 0x4ed4f6d in file_close vfs/fs_close.c:78
    #39 0x4ed1459 in nx_close_from_tcb inode/fs_files.c:754
    #40 0x4ed1501 in nx_close inode/fs_files.c:781
    #41 0x4ed154a in close inode/fs_files.c:819
    #42 0x6bcb9ce in property_get kvdb/client.c:307
    #43 0x6bcd465 in property_get_int32 kvdb/common.c:270
    #44 0x5106c9a in tz_offset_restore app/miwear_bluetooth.c:745
    #45 0x510893f in miwear_bluetooth_main app/miwear_bluetooth.c:1033
    #46 0x4dcf5c8 in nxtask_startup sched/task_startup.c:70
    #47 0x4d70873 in nxtask_start task/task_start.c:134
    #48 0x4e04a07 in pre_start sim/sim_initialstate.c:52

Signed-off-by: ligd <liguiding1@xiaomi.com>
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.

1 participant