Skip to content

Revert commits from PR #2469#2474

Closed
tmedicci wants to merge 1 commit into
apache:masterfrom
tmedicci:bugfix/revert_pr_2469
Closed

Revert commits from PR #2469#2474
tmedicci wants to merge 1 commit into
apache:masterfrom
tmedicci:bugfix/revert_pr_2469

Conversation

@tmedicci

@tmedicci tmedicci commented Aug 9, 2024

Copy link
Copy Markdown
Contributor

Summary

echo $? does not return any exit code. This is largely used by automated testing. This PR reverts it until it is properly fixed.

Revert commits from PR #2469

Revert "nsh_fileapp: handle input redirection"

This reverts commit e46347e.

Revert "feat(nsh_cat): allow cat to read from stdin"

This reverts commit 8fba726.

Revert "feat(nsh): add console read"

This reverts commit 4104019.

Revert "feat(nsh): input (stdin) redirection"

This reverts commit 96100b3.

Impact

Fix echo $? bug.

Testing

nsh> ls
/:
 dev/
 proc/
nsh> echo $?
0

Internal CI testing (HW testing with all supported Espressif SoCs)

Revert "nsh_fileapp: handle input redirection"

This reverts commit e46347e.

Revert "feat(nsh_cat): allow cat to read from stdin"

This reverts commit 8fba726.

Revert "feat(nsh): add console read"

This reverts commit 4104019.

Revert "feat(nsh): input (stdin) redirection"

This reverts commit 96100b3.
@tmedicci tmedicci mentioned this pull request Aug 9, 2024
@acassis acassis marked this pull request as draft August 9, 2024 15:31
@acassis

acassis commented Aug 9, 2024

Copy link
Copy Markdown
Contributor

I will submit a fix to lm3s6965 board

@acassis

acassis commented Aug 9, 2024

Copy link
Copy Markdown
Contributor

This PR will fix the issue: apache/nuttx#12873

@tmedicci

tmedicci commented Aug 9, 2024

Copy link
Copy Markdown
Contributor Author

Hi @acassis , this PR is not related to the lm3s6965 board. Please, reconsider reviewing it and reverting ASAP.

@tmedicci

tmedicci commented Aug 9, 2024

Copy link
Copy Markdown
Contributor Author

Just to make it clear, @acassis : #2469 make echo $? return nothing even on sim:nsh (all Espressif's defconfig included). it isn't a matter of a specific configuration.

@casaroli

casaroli commented Aug 9, 2024

Copy link
Copy Markdown
Contributor

I will do some testing to verify this

@acassis

acassis commented Aug 9, 2024

Copy link
Copy Markdown
Contributor

@tmedicci

tmedicci commented Aug 9, 2024

Copy link
Copy Markdown
Contributor Author

@tmedicci understood in fact the CI still breaking:

https://github.com/apache/nuttx/actions/runs/10322320078/job/28577343134?pr=12873

yes, @acassis . It's breaking not because of the lm3s6965 board. This PR's check finished successfully. Again, please, accept this PR until we have the issue of #2469 solved to stop the CI from failing.

@tmedicci tmedicci marked this pull request as ready for review August 9, 2024 18:43
@acassis

acassis commented Aug 9, 2024

Copy link
Copy Markdown
Contributor

@tmedicci I understood, even the CI is failing:

/github/workspace/sources/nuttx/tools/ci/testrun/script/test_open_posix/test_openposix_.py:9703: AssertionError
----------------------------- Captured stdout call -----------------------------
+------------------------------------------- Catch Exception -------------------------------------------+
+----------------------------------------------- Result ------------------------------------------------+
| Command     : 'echo $?'                                                                               |
| Expect value: ['0']                                                                                   |
| Timeout     : 2s                                                                                      |
| Test result : Timeout                                                                                 |
+-------------------------------------------------------------------------------------------------------+
_____________________ test_ltp_interfaces_sigaction_12_39 ______________________

@casaroli

Copy link
Copy Markdown
Contributor

Let me try to fix it without reverting. I am doing some testing now.

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.

3 participants