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

Refactor actions script #132

Merged
merged 12 commits into from
Jan 18, 2023
Merged

Refactor actions script #132

merged 12 commits into from
Jan 18, 2023

Conversation

Jamm02
Copy link
Collaborator

@Jamm02 Jamm02 commented Jan 18, 2023

No description provided.

- master
paths:
- '.github/workflows/test-install-script.yml'
- 'wall_e_install.sh'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed

- master
paths:
- '.github/workflows/test-install-script.yml'
- 'wall_e_install.sh'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed

include:
- name: MacOS-latest
host_runner: macos-latest
package_manager: brew
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed

Comment on lines 11 to 13
paths:
- '.github/workflows/test-install-script.yml'
- 'wall_e_install.sh'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be removed

Comment on lines 17 to 19
paths:
- '.github/workflows/test-install-script.yml'
- 'wall_e_install.sh'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be removed

@SAtacker
Copy link
Member

@SAtacker
Copy link
Member

Add at the top of walle_install.sh

set -e

Exit immediately if a pipeline (see Pipelines), which may consist of a single simple command (see Simple Commands), a list (see Lists of Commands), or a compound command (see Compound Commands) returns a non-zero status.

@SAtacker SAtacker added github-ci Issues related to GitHub CI installer Install script related issues labels Jan 18, 2023
@SAtacker
Copy link
Member

@SAtacker
Copy link
Member

SAtacker commented Jan 18, 2023

@Jamm02

Define a _sudo function

_sudo(){
    if ! command -v sudo &> /dev/null
    then
        if ! [ $(id -u) = 0 ]; then
           echo "The script need to be run as root." >&2
           exit 1
        fi
        "$@"
    else
        sudo "$@"
    fi
}

Replace sudo with _sudo

@@ -27,7 +27,6 @@ unameOut="$(uname -s)"
case "${unameOut}" in
Linux*)
_sudo apt update && _sudo apt upgrade -y
_sudo usermod -a -G dialout $USER
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of removing, try double quoting the "$USER"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was intended to be removed (#117) so that participants would be exposed to enabling port access.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, sorry use [ ! -z "${USER}" ] && _sudo usermod -a -G dialout "$USER"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, thanks

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not know that

Comment on lines +29 to +30
_sudo apt update && _sudo apt upgrade -y
_sudo apt install git wget flex bison gperf python3 python3-pip python3-setuptools cmake ninja-build ccache libffi-dev libssl-dev dfu-util libusb-1.0-0 -y
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export DEBIAN_FRONTEND=noninteractive

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and then do apt stuff

@SAtacker
Copy link
Member

MacOs has issues due to a python 11 package. Ref - espressif/esp-idf#10116 (comment)

@SAtacker SAtacker self-requested a review January 18, 2023 20:40
@SAtacker SAtacker merged commit 3490c98 into SRA-VJTI:master Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github-ci Issues related to GitHub CI installer Install script related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants