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

Make export.fish exit with SUCCESS exit code (IDFGH-9460) #10828

Closed
wants to merge 3 commits into from
Closed

Make export.fish exit with SUCCESS exit code (IDFGH-9460) #10828

wants to merge 3 commits into from

Conversation

maxslarsson
Copy link
Contributor

@maxslarsson maxslarsson commented Feb 22, 2023

The export.fish script currently exits with an exit code of 4. Thus, any checks that make sure the source command exited successfully (exit code equal to zero) always fail. This was due to the last line trying to erase the __main function using the set -e command. In fish, you can only erase variables using the set command. By changing the last line to erase the __main function with the functions -e command, the script now exits with an exit code of 0 instead of 4.

Also, while looking at the script, I noticed that the unset function seemed to be unused. My understanding was that the author of the script opted to directly use the set -e command in the script instead of the unset function, leaving it unused.

The export.fish script exits with an exit code of 4. Thus, any shell checks that make sure the source command exits successfully always failed. This was due to the last line trying to erase the __main function. In fish, you can't erase a function using the `set` command, you can only erase variables. By removing that line the script now exits with an exit code of 0 instead of 4.
@CLAassistant
Copy link

CLAassistant commented Feb 22, 2023

CLA assistant check
All committers have signed the CLA.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Feb 22, 2023
@github-actions github-actions bot changed the title Make export.fish exit with SUCCESS exit code Make export.fish exit with SUCCESS exit code (IDFGH-9460) Feb 22, 2023
Remove the unused `unset` function in export.fish. The author of the script decided to simply use `set -e` instead so the function was never used
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

@espressif-bot espressif-bot added Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed and removed Status: In Progress Work is in progress labels Mar 1, 2023
@mfialaf
Copy link
Collaborator

mfialaf commented Mar 8, 2023

@maxslarsson Thanks for your contribution!
Your change about erasing the __main function works well, thanks for that. I squashed those commits together, so it is merged as one change.

On the other hand, I had to remove you commit Remove unused unset function in export.fish. This function is used when users are switching from one ESP-IDF version to another in opened terminal. One of subcommand in export script then generate sequence of commands with keyword unset for unsetting global variables from the previous version. unset is accepted in all other shells but not fish. Because of that, the function is needed in export.fish.

@maxslarsson
Copy link
Contributor Author

Ahh I see! Thank you for letting me know and for taking the time to review my changes. Let me know if I can do anything else!

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Reviewing Issue is being reviewed Resolution: NA Issue resolution is unavailable labels Mar 16, 2023
@maxslarsson maxslarsson deleted the patch-1 branch March 17, 2023 13:19
espressif-bot pushed a commit that referenced this pull request Mar 19, 2023
The export.fish script exits with an exit code of 4. Thus, any shell checks that make sure the source command exits successfully always failed. This was due to the last line trying to erase the __main function. In fish, you can't erase a function using the `set` command, you can only erase variables. By removing that line the script now exits with an exit code of 0 instead of 4.

Erase __main function at the end of export.fish

Closes #10828
espressif-bot pushed a commit that referenced this pull request Mar 20, 2023
The export.fish script exits with an exit code of 4. Thus, any shell checks that make sure the source command exits successfully always failed. This was due to the last line trying to erase the __main function. In fish, you can't erase a function using the `set` command, you can only erase variables. By removing that line the script now exits with an exit code of 0 instead of 4.

Erase __main function at the end of export.fish

Closes #10828
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Status: Opened Issue is new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants