-
Notifications
You must be signed in to change notification settings - Fork 0
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
SECZ-1582: Add Linux support #18
Conversation
|
||
# If using an M1 machine, load shell environment to run brew commands | ||
if [[ $(uname -m) == 'arm64' ]]; then | ||
echo ‘# Set PATH, MANPATH, etc., for Homebrew.’ >> ~/.zprofile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unicode Quote -> ASCII quote
79b95a5
to
d3a2746
Compare
setup.sh
Outdated
eval "$(/opt/homebrew/bin/brew shellenv)" | ||
fi | ||
|
||
if [[ "$kernel_name" == "Linux" ]]; then | ||
(echo; echo 'eval "$(/home/linuxbrew/.linuxbrew/bin/brew shellenv)"') >> ~/.bashrc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to modify ~/.bashrc? I know it looks like we did something similar with the mac version of the script. But if we're only loading shell env vars to be able to run "brew" commands for the duration of this script, is it really necessary to modify our default shell behavior?
Could we just drop this line and use eval "$(/home/linuxbrew/.linuxbrew/bin/brew shellenv)"
below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also Linuxbrew is not installed currently on the eng image. How can we ensure that linuxbrew is installed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is eval "$(/home/linuxbrew/.linuxbrew/bin/brew shellenv)"
sets up the current shell session to run brew, so you need to add it to .bashrc
for it to apply to every shell session
An overall comment: How are you imagining this is run? As an |
setup.sh
Outdated
(echo; echo 'eval "$(/home/linuxbrew/.linuxbrew/bin/brew shellenv)"') >> ~/.bashrc | ||
eval "$(/home/linuxbrew/.linuxbrew/bin/brew shellenv)" | ||
sudo apt install -y build-essential | ||
brew install gcc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we do sudo apt install gcc
instead? If we can install gcc without brew, we may not need linuxbrew
at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could. Actually this code is unreachable and we could just remove it outright too -- the Brew installation if check above first checks if we're on Mac OS, so if we're on Linux Brew doesn't get installed and this code doesn't run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need Linuxbrew for other purposes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't run anything setting up the eng image that required Linuxbrew.
Any EDIT: I remember that we have the uninstall process here for testing the setup from scratch. That would require reinstalling the |
# Remove leapp | ||
sudo dpkg -r leapp | ||
sudo dpkg -P leapp | ||
# Remove session-manager-plugin | ||
sudo dpkg -r session-manager-plugin | ||
sudo dpkg -P session-manager-plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you combine the remove and purge flags to run this once for each package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, dpkg complains if you do
This runs as the
But this doesn't actually get run. I could add move it to a spot where it would get run |
d7ecb71
to
a054497
Compare
Testing:
Core objective: make this script work on both Linux & Mac
Additional tweaks: