xt-sys/exectos

Fork 3

POSIX compliant stream redirection #1

Merged

belliash merged 3 commits from :fix-img-generation-bug into master

2023-11-23 17:02:30 +01:00

Ghost commented

2023-11-20 00:26:20 +01:00

First-time contributor

Avoid background processing for "dash" shell race condition bug.

Purpose

On certain systems a race condition can happen when using & (background).

This is because CMake won't wait until the command finishes as & will just let it run on background and ignore finishing.

Proposed changes

These tweaks

Avoid background processing for "dash" shell race condition bug. ## Purpose On certain systems a race condition can happen when using & (background). This is because CMake won't wait until the command finishes as & will just let it run on background and ignore finishing. ## Proposed changes These tweaks

Ghost added 1 commit 2023-11-20 00:26:21 +01:00

chore(cmake): refine disk image creation in qemu.cmake fb963d7b69

Avoid background processing for "dash" shell race condition bug.

belliash commented

2023-11-20 07:34:24 +01:00

Owner

This command is NOT terminated by the control operator '&', so the shell does NOT execute the command in the background in a subshell.

Modified command line contains originally '&>' at the end. This construction redirects STDOUT and STDERR to /dev/null, so that there is no output from 'dd' command shown. Please check manual: https://www.gnu.org/software/bash/manual/bash.html#Redirecting-Standard-Output-and-Standard-Error

You can add echo "$SHELL" or ps -p $$ somewhere to check if this is really a dash. If it is, then I would check what launches it and think how to prevent that, because XTChain build environment should startup in bash (what you can confirm or deny with same command). It is possible, that CMake spawns dash as it is your default shell.

This command is NOT terminated by the control operator '&', so the shell does NOT execute the command in the background in a subshell. Modified command line contains originally '&>' at the end. This construction redirects STDOUT and STDERR to /dev/null, so that there is no output from 'dd' command shown. Please check manual: https://www.gnu.org/software/bash/manual/bash.html#Redirecting-Standard-Output-and-Standard-Error You can add `echo "$SHELL"` or `ps -p $$` somewhere to check if this is really a dash. If it is, then I would check what launches it and think how to prevent that, because XTChain build environment should startup in bash (what you can confirm or deny with same command). It is possible, that CMake spawns dash as it is your default shell.

Ghost commented

2023-11-20 07:41:01 +01:00

Author

First-time contributor

@belliash This is indeed an issue of dash, as the &>/dev/null statement isn't POSIX compliant.

If it is, then I would check what launches it and think how to prevent that, because XTChain build environment should startup in bash (what you can confirm or deny with same command).

This is true, the issue is in the target recipe itself.
Some commands in the diskimg target are prepended with sh -c, where sh is a symlink to another shell binary (dash is the default symlink on Debian-based distributions, whereas Gentoo/Arch have bash linked as default).

Possible fixes are to either switch to a POSIX-compliant output redirection (1>/dev/null) or remove the sh invokation.

@belliash This is indeed an issue of dash, as the `&>/dev/null` statement isn't POSIX compliant. > If it is, then I would check what launches it and think how to prevent that, because XTChain build environment should startup in bash (what you can confirm or deny with same command). This is true, the issue is in the target recipe itself. Some commands in the diskimg target are prepended with `sh -c`, where `sh` is a symlink to another shell binary (dash is the default symlink on Debian-based distributions, whereas Gentoo/Arch have bash linked as default). Possible fixes are to either switch to a POSIX-compliant output redirection (1>/dev/null) or remove the `sh` invokation.

belliash requested review from belliash 2023-11-20 07:59:49 +01:00

belliash self-assigned this 2023-11-20 07:59:52 +01:00

belliash commented

2023-11-20 08:04:13 +01:00

Owner

Some commands in the diskimg target are prepended with sh -c, where sh is a symlink to another shell binary (dash is the default symlink on Debian-based distributions, whereas Gentoo/Arch have bash linked as default).

This is true as sh is just a symlink to 'some' shell frequently.

Possible fixes are to either switch to a POSIX-compliant output redirection (1>/dev/null) or remove the sh invokation.

This is true again, but I believe this should be 1>/dev/null 2>/dev/null as &> redirects both stdout and stderr and the meaning of this PR is to fix disk image creation under dash, not to change script behavior.

>Some commands in the diskimg target are prepended with sh -c, where sh is a symlink to another shell binary (dash is the default symlink on Debian-based distributions, whereas Gentoo/Arch have bash linked as default). This is true as sh is just a symlink to 'some' shell frequently. > Possible fixes are to either switch to a POSIX-compliant output redirection (1>/dev/null) or remove the `sh` invokation. This is true again, but I believe this should be `1>/dev/null 2>/dev/null` as `&>` redirects both stdout and stderr and the meaning of this PR is to fix disk image creation under dash, not to change script behavior.

Ghost added 1 commit 2023-11-20 16:43:11 +01:00

chore(cmake): Disable stdout/err output for diskimg target 4c7d23000e

belliash changed title from ~~Tweak disk image creation in qemu.cmake~~ to POSIX compliant stream redirection

2023-11-23 08:20:20 +01:00

belliash approved these changes 2023-11-23 16:38:21 +01:00

Ghost force-pushed fix-img-generation-bug from 744e4e30cc to 4c7d23000e

2023-11-23 16:57:16 +01:00

Compare

Ghost added 1 commit 2023-11-23 16:58:27 +01:00

Merge branch 'master' into fix-img-generation-bug 656ddbd5ff

belliash merged commit a01ccff6dc into master

2023-11-23 17:02:30 +01:00