-
Notifications
You must be signed in to change notification settings - Fork 35
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
Added Debian bullseye support for the installer #1202
base: main
Are you sure you want to change the base?
Conversation
Merge from upstream
Thanks for submitting this @marnunez, quick feedback:
|
|
@marnunez If you fix the formatting, and @halleysfifthinc approves the change (to make sure it doesn't break his changes, I don't have access to either platforms) that would be great. Also please include in this PR, adding your name to the list of contributors in the contributing.md file. Thank you |
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.
The added whitespace will need to be removed or else it will show up in the printf statements and mess with the output indenting.
Comment=OpenSim is an open source software for neuromusculoskeletal modeling, simulation \ | ||
and analysis. |
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.
Comment=OpenSim is an open source software for neuromusculoskeletal modeling, simulation \ | |
and analysis. | |
Comment=Neuromusculoskeletal modeling, simulation, and analysis. |
We had changed this in the standalone desktop file but I noticed that I had forgot to make that change here as well.
|
||
if [[ $OS == "Ubuntu" ]] && [[ $VER == "18.04" ]]; then | ||
fi | ||
if ([[ $OS == "ubuntu" ]] && [[ $VER == "18.04" ]]) || ([[ $OS == "debian" ]] && [[ $OS_FULL =~ "bullseye" ]]); then |
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.
Is there a reason (ie in terms of an expectation of a non-exact match) for using the =~
comparison for matching "bullseye"? I'm not familiar with Debian
if ! [ -z "${VERSION_ID+x}" ]; then | ||
VER=$VERSION_ID | ||
fi |
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 bash skills aren't the greatest (and I wasn't able to figure it out with some quick googling); can you explain what the +x
is doing? Also, if you're negating the -z
option, wouldn't using -n
or even just [ "$VERSION_ID+x}" ]
be more concise/readable?
VER=$VERSION_ID | ||
. /etc/os-release | ||
OS=$ID | ||
OS_NAME=$NAME |
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.
This variable appears to be unused now and can be removed?
Regarding the Actions CI artifacts, I believe what @aymanhab is suggesting is to check that the installer produced by CI works by test installing it on your machine. |
I have run the build script under Debian bullseye and found 1 issue : However, the installation crashes very easily including on the examples. Not sure if this is a problem with any older libs included in debian |
Brief summary of changes
Changed to use ID of OS instead of NAME
Only use VERSION_ID if ID == "ubuntu"
Testing I've completed
Built on my own Debian bullseye/sid