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.

Sign up for GitHub

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

Print out warning only if platform is not Ubuntu and make build directory relative to where build starts #382

Closed
wants to merge 5 commits into from
Closed

Print out warning only if platform is not Ubuntu and make build directory relative to where build starts #382

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b1a69e
Print out warning only if platform is not Ubuntu and make build direc…
linh2931 Jun 14, 2022
06c993d
Use uname -a from -v and add quotes around the command
linh2931 Jun 14, 2022
1805ba5
Merge branch 'main' into pinned_build_sh_fix
linh2931 Jun 15, 2022
4108f6c
Use os-release to determine the operating system and print out exact …
linh2931 Jun 15, 2022
c87e56c
Merge branch 'pinned_build_sh_fix' of https://github.com/eosnetworkfo…
linh2931 Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion scripts/pinned_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

echo "Mandel Pinned Build"

if [[ $NAME != "Ubuntu" ]]
if [[ $(uname -v) != *Ubuntu* ]]
Copy link
Member

Choose a reason for hiding this comment

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

fwiw this may not yield expected behavior when running ubuntu in a container. Also, how safe is it for uname -v to not be in quotes here. Locally I see spaces in that

$ uname -v
#1 SMP PREEMPT_DYNAMIC Thu, 09 Jun 2022 16:14:10 +0000

Copy link

Choose a reason for hiding this comment

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

I was expecting something more like:

#!/bin/sh

if [ -e /etc/os-release ]
then
        . /etc/os-release
fi

echo $ID

Copy link
Member Author

@linh2931 linh2931 Jun 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks Matt. I will add quotes there.

I was testing inside a Ubuntu container built on VirtualBox on my PC

uname -v
#49~20.04.1-Ubuntu SMP Wed May 18 18:44:28 UTC 2022

I will change to do uname -a to be safe.

Copy link

@matthewdarwin matthewdarwin Jun 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This will not work. I am running a Ubuntu lxc container on a debian host. uname -a shows Debian. It is not debian (only the kernel is Debian).

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @matthewdarwin. I have changed to use os-release and provide specific reason when a build is not support.

then
echo "Currently only supporting Ubuntu based builds. Proceed at your own risk."
fi
Expand Down Expand Up @@ -113,6 +113,8 @@ install_clang ${DEP_DIR}/clang-${CLANG_VER}
install_llvm ${DEP_DIR}/llvm-${LLVM_VER}
install_boost ${DEP_DIR}/boost_${BOOST_VER//\./_}

popdir ${SCRIPT_DIR}

pushdir ${MANDEL_DIR}

# build Mandel
Expand Down