Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Fix detecting MAJOR_VERSION in linux #646

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix detecting MAJOR_VERSION in linux #646

wants to merge 2 commits into from
+1 −1

Conversation

417-72KI
Copy link
Member

@417-72KI 417-72KI commented Mar 16, 2025 •
edited
Loading

When swift --version returns like 6.0.3 installation script fails to get MAJOR_VERSION and modules are not copied into LIB_INSTALL_PATH.

@417-72KI 417-72KI marked this pull request as ready for review March 16, 2025 18:06
f-meloni
f-meloni reviewed Mar 16, 2025
View reviewed changes
Scripts/install.sh Outdated
@@ -20,7 +20,7 @@ if [[ "$OSTYPE" == "darwin"* ]]; then
else
BUILD_FOLDER=".build/debug"
swift build --disable-sandbox
MAJOR_VERSION=$(swift --version | awk '{for(i=1;i<=NF;i++){if($i ~ /^[0-9]+\.[0-9]+$/){print $i; break}}}' | cut -d '.' -f 1)
MAJOR_VERSION=$(swift --version | awk '{for(i=1;i<=NF;i++){if($i ~ /^[0-9]+\.[0-9]+\.[0-9]?$/){print $i; break}}}' | cut -d '.' -f 1)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a regex expert, but shouldn't this be (.[0-9]+)? To have all the group optional and support more than one digit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Certainly, thank you.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed!

@417-72KI
Copy link
Member Author

@f-meloni Do you have any other concerns to merge this?

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@f-meloni f-meloni f-meloni approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@417-72KI @f-meloni